-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
introduce MixedMesh class #303
base: main
Are you sure you want to change the base?
introduce MixedMesh class #303
Conversation
0eeb02f
to
71d79f4
Compare
ufl/algorithms/analysis.py
Outdated
base_coeff_and_args = extract_type(a, (BaseArgument, BaseCoefficient)) | ||
arguments = [f for f in base_coeff_and_args if isinstance(f, BaseArgument)] | ||
coefficients = [f for f in base_coeff_and_args if isinstance(f, BaseCoefficient)] | ||
base_coeff_and_args_and_gq = extract_type(a, (BaseArgument, BaseCoefficient, GeometricQuantity)) |
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.
Could we just call this list base_types
, as it is quite explicit from the function call what it does, and it makes the latter code more readable.
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.
Renamed it terminals
.
if not isinstance(gq._domain, Mesh): | ||
raise TypeError(f"{gq}._domain must be a Mesh: got {gq._domain}") |
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.
What else com gq._domain
be? is this relying on the deprecated definition that a domain
can be defined just through a ufl.Cell
?
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. It checks if gq._domain
is Mesh
and not MixedMesh
.
domain = MixedMesh(mesh0, mesh1, mesh2) | ||
V = FunctionSpace(domain, elem) |
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.
So we now have two ways of creating a FunctionSpace
with MixedElement
s.
- A single mesh of class
Mesh
with aMixedElement
, whereMixedElement
can have arbitrary many elements - A
MixedMesh
consisting ofM
meshes, and aMixedElement
consisting ofM
elements.
Do we need additional checks in the FunctionSpace
constructor to ensure that this is satisfied (instead of catching this at a later instance, inside for instance GenericDerivativeRuleset.reference_value
.
I would also like to highlight that the API for MixedElement
and MixedMeshes
differs, as one passes a list, while the other unrolls a list, I think we should be consistent, and use lists in both places
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.
Added checks in FunctionSpace
constructor and changed MixedMesh
API as suggested. Thanks.
ufl/algorithms/apply_derivatives.py
Outdated
if element.num_sub_elements != len(domain): | ||
raise RuntimeError(f"{element.num_sub_elements} != {len(domain)}") | ||
g = ReferenceGrad(o) | ||
vsh = g.ufl_shape[:-1] |
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.
Could we give this a more obvious name?
Isn't this isn't this just the ufl_shape
of the input operand, which should always be the same as the ufl_shape
of o
.
Another question (as I don't quite see how this works) is what is the ref_dim
?, which would be the topological dimension of the unique "ufl-domain" of o
. But what is the topological dimension of a MixedMesh
?
Does this mean that a MixedMesh
cannot have co-dimension 1 meshes in them, i.e. a Mesh of triangles and a Mesh of intervals?
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.
Turned out vsh
could be removed, so I did.
You are right. Currently, MixedMesh
can only be composed of meshes of the same cell_type, so topological dimension is well defined. I edited the doc string of MixedMesh
to make this point clear.
It would be helpful to have a PR summary, e.g. what it sets out the achieve, what it supports, designs considered, design rationale, implementation summary, and what a MixedMesh is and how a MixedMesh is different from a Mesh. |
6dbbd72
to
3512b14
Compare
Updated PR summary. Added more checks. Fixed |
3512b14
to
510af2f
Compare
Can I have another round of review on this? |
First half of #264
Introduce
MixedMesh
class for multi-mesh problems.Note that
extract_domains()
now uses more robustsort_domains()
inside, so it might behave slightly differently.Edit 12-09-2024:
MixedMesh
class represents a collection of meshes (e.g., submeshes) that, along with aMixedElement
, can represent a mixed function space defined across multiple domains.The motivation is to allow for treating
Argument
s andCoefficient
s on a mixed function space defined across multiple domains just like those on a mixed function space defined on a single mesh.Specifically, the following becomes possible (see tests for more):
For now, one can only perform cell integrations when
MixedMesh
es are used. This is because, e.g., an interior facet integration on a submesh may either be interior or exterior facet integration on the parent mesh, and we need a logic to apply default restrictions on coefficients defined on the participating meshes. This is the second half of #264.Also, currently, all component meshes must have the same cell type (and thus the same topological dimension) -- we are to remove this limitation in the future.
Core changes:
GradRuleSet.{reference_value, reference_grad}
work component-wise (component of the mixed space) if theFunctionSpace
is defined on aMixedMesh
, so that each component is associated with a component of theMixedMesh
, saydomain_i
(JacobianInverse(domain_i)
is then well defined).extract_arguments_and_coefficients
is now calledextract_terminals_with_domain
, and it now also collectsGeometricQuanty
s, so that we can correctly handle, saySpatialCoordinate(mesh1)
andSpatialCoordinate(mesh2)
, in problem solving environments.