-
-
Notifications
You must be signed in to change notification settings - Fork 75
UniqueDomainExtractor DAG traverser
#412
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?
UniqueDomainExtractor DAG traverser
#412
Conversation
connorjward
left a comment
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 wonder if this could be generalised a little to a "domain extractor" that returns all domains. extract_unique_domain is then a special case of that.
001dfe0 to
298846c
Compare
fixes
298846c to
e296ddc
Compare
|
|
||
| if isinstance(expression_domain, MeshSequence): | ||
| index = multiindex[0]._value | ||
| element = expression.ufl_element() |
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.
This assumes that o.ufl_operands[0] has ufl_element(), so something like the following will fail:
cell = triangle
elem0 = LagrangeElement(cell, 1)
elem1 = LagrangeElement(cell, 1)
elem = MixedElement([elem0, elem1])
mesh1 = Mesh(LagrangeElement(cell, 1, (2,)), ufl_id=100)
mesh2 = Mesh(LagrangeElement(cell, 1, (2,)), ufl_id=101)
domain = MeshSequence([mesh1, mesh2])
V = FunctionSpace(domain, elem)
f = Coefficient(V)
d = extract_unique_domain(
Indexed(
grad(f),
MultiIndex((FixedIndex(0), FixedIndex(1))),
)
)
It's probably hard to make postorders robustly handle this sort of things.
First alternative that comes to my mind is preorder (or whatever you call it):
@process.register(Indexed)
def _(self, o, multiindex):
expression, multiindex = o.ufl_operands
return self(expression, multiindex=multiindex)
@process.register(Grad)
def _(self, o, multiindex):
return self(o.ufl_operands[0], multiindex[:-1])
... and so on
You can see https://github.com/FEniCS/ufl/blob/main/ufl/algorithms/apply_coefficient_split.py.
CoefficientSplitter works as we make sure that ReferenceValue, ReferenceGrad, and Restricted have already been propagated to directly wrap terminals by the time we use it; you might need some "balancing" of this kind before calling extract_unique_domain, too, but it could be inconvenient.
Do you only need this for indexed Arguments and Coefficients? What is the most general use case in your mind?
| from ufl.constant import Constant | ||
| from ufl.geometry import GeometricQuantity | ||
|
|
||
| if isinstance(o, Coefficient | Argument): |
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 define separate singledispatch method as:
@process.register(Coefficient)
@process.register(Argument)
def _(self, o, ...):
...
Same for the other types.
| V1 = FunctionSpace(mesh1, scalar_elem) | ||
| V2 = FunctionSpace(mesh2, scalar_elem) | ||
|
|
||
| A = Matrix(V1, V2) | ||
| assert extract_unique_domain(A) == mesh1 |
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 this what we expect?
| def extract_unique_domain(expr, expand_mesh_sequence: bool = True): | ||
| """Return the single unique domain expression is defined on or throw an error. | ||
| Args: | ||
| expr: Expr or Form. | ||
| expand_mesh_sequence: If True, MeshSequence components are expanded. | ||
| Returns: | ||
| domain. | ||
| """ | ||
| domains = extract_domains(expr, expand_mesh_sequence=expand_mesh_sequence) | ||
| if len(domains) == 1: | ||
| return domains[0] | ||
| elif domains: | ||
| raise ValueError("Found multiple domains, cannot return just one.") | ||
| else: | ||
| return None | ||
|
|
||
|
|
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.
Can we not change the definition of extract_unique_domain() here? Instead, can we change
for t in traverse_unique_terminals(expr):
domainlist.extend(t.ufl_domains())
in extract_domains() function to use DAGTraverser? I think it is important to make sure that extract_domains() and extract_unique_domains() are consistent.
Extract unique domain from an expression by traversing the expression tree. If we reach a terminal, we return the domain of that. If we reach an operator, we return the domain of the operands, if they all match.
Unlike the previous
unique_domain_extractor, the DAG traverser works with expressions containing indexed arguments and coefficients. For example,I plan to extend this in the future so that it works with
BaseForms containing indexed arguments and coefficients, though I think this will require implementing anIndexedBaseFormclass