-
-
Notifications
You must be signed in to change notification settings - Fork 71
Introduce MeshSequence + handle restrictions on MeshSequence #264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Introduce MeshSequence + handle restrictions on MeshSequence #264
Conversation
e07f9fa
to
d574909
Compare
e66d41d
to
d3e72fb
Compare
23696eb
to
ae55b3b
Compare
ae55b3b
to
4ced3a7
Compare
7e4dd44
to
ef8fd99
Compare
restriction = t._side | ||
t, = t.ufl_operands | ||
elif t._ufl_terminal_modifiers_: | ||
raise ValueError("Missing handler for terminal modifier type %s, object is %s." % (type(t), repr(t))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for not using f-strings
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I will fix it when I tidy this up. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
# Only split mixed coefficients | ||
return o | ||
# Reference value expected | ||
assert reference_value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this throw a run-time error instead of an assertion, as something has clearly gone wrong if you have gotten here without t
being a Coefficient and ReferenceValue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, fixed.
988189f
to
36559fc
Compare
36559fc
to
9317b5b
Compare
9317b5b
to
883ee0b
Compare
Should this PR be closed in favour of #303? Or this a different feature? |
883ee0b
to
2d76a5e
Compare
3f5f1ea
to
b2f2fec
Compare
f55fd36
to
5882239
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm basically happy with this.
ufl/measure.py
Outdated
@@ -122,6 +130,8 @@ def __init__( | |||
affecting how code is generated, including parameters | |||
for optimization or debugging of generated code | |||
subdomain_data: object representing data to interpret subdomain_id with | |||
extra_measures: additional domain-integral_type map | |||
for multi-domain problems |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs serious documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
ufl/algorithms/signature.py
Outdated
@@ -143,6 +143,9 @@ def compute_form_signature(form, renumbering): # FIXME: Fix callers | |||
integrand_hashdata = compute_expression_hashdata(integral.integrand(), terminal_hashdata) | |||
|
|||
domain_hashdata = integral.ufl_domain()._ufl_signature_data_(renumbering) | |||
extra_measures_hash_data = tuple( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra_measures_hash_data = tuple( | |
extra_measures_hashdata = tuple( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
ufl/algorithms/signature.py
Outdated
@@ -153,6 +156,7 @@ def compute_form_signature(form, renumbering): # FIXME: Fix callers | |||
integrand_hashdata, | |||
domain_hashdata, | |||
integral.integral_type(), | |||
extra_measures_hash_data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra_measures_hash_data, | |
extra_measures_hashdata, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@jorgensd I will touch up slightly more today, but could you have a look at this PR? |
fb5f4a4
to
abc62d6
Compare
Updated the description. |
5350b9f
to
1b26a8f
Compare
1b26a8f
to
5650c45
Compare
5650c45
to
6ec0d57
Compare
Depend on #382 (small).
In multi-domain (submesh) problems, there are situations where, say, an exterior-facet integration of domainA is an interior-facet integration of domainB. In this PR we extend the
Measure
class to correctly represent such integration. Specifically, in the above example, if we choosedomainA
as the "primal" integration domain, we define the interested measure as:. This required changing:
form.py
),IntegralData
grouping (algorithms/domain_analysis.py
),signature
(algorithms/signature.py
),accordingly.
We also modified
RestrictionPropagator
class andapply_restrictions()
function.apply_restriction()
function now takesdefault_restrictions
kwarg, so that we can specify the default restrictions domain-wise. In the above case, we will have something like:.