Skip to content
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

Fix acquisition of dof lists from Domains #1275

Merged
merged 13 commits into from
Nov 27, 2024

Conversation

btalamini
Copy link
Member

@btalamini btalamini commented Nov 22, 2024

There are two sets of books in Domain:

  • Element index lists that are broken out by element geometry, answering questions like "for this tetrahedron in the Domain, where does it go in the list of all tetrahedra in the mesh?". This is used by Functional to identify entities that should be integrated over.
  • Global element indices, for answering "which element is this in the mfem::Mesh data structure?" (which does not segregate by element geometry). These are used to be able to grab dofs that live on a Domain.

As noted in #1272, some of the factory methods and operators on Domain did not populate all the required data. The omissions were the second group, the global element data, so grabbing dofs were broken in some cases.

Fundamentally, this class should enforce that every time an entry is added to the first group, a corresponding entry is also added to the second. That's an interface-breaking change I don't want to undertake now. I fixed the bug and moved the class one step closer to the ideal by adding addElement(...) and addElements(...) methods that require both indices to be supplied at once. In sum this PR does the following:

  • I changed all the factory methods to use the addElement(...) and addElements(...) methods, which fixes the bug, and sets a pattern that makes it less likely a modifier of this class will do the wrong thing.
  • I added documentation to discourage a future modifier from manipulating the lists individually
  • I added tests that would have caught the missing functionality.

Fixes #1272.

@samuelpmishLLNL
Copy link
Contributor

/style

@samuelpmishLLNL samuelpmishLLNL merged commit ad3c70a into develop Nov 27, 2024
2 checks passed
@btalamini btalamini deleted the talamini/bugfix/domain branch November 28, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some factory methods for `Domain' don't fully construct valid objects
3 participants