-
Notifications
You must be signed in to change notification settings - Fork 301
Simple mesh fixes #4380
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: devel
Are you sure you want to change the base?
Simple mesh fixes #4380
Conversation
This wasn't failing the test, because the kernel here didn't need the unprepared data, but we'd like to simply assert that we're not assembling or solving on unprepared meshes.
Otherwise the mesh still thinks its fully prepared even when it isn't and we can't assert valid_is_prepared() on it.
We've added some tricky interior_mesh options that our copy_nodes_and_elements code wasn't handling correctly, and it was possible for a clone() of such a mesh to fail its operator== assertion.
This must have been an atavism from a previous refactoring.
We never caught this before because we were never doing much subsequent mesh modification to the output, but as soon as we start heavier testing we run into code that screams about the inconsistent parent/child id ordering.
This was supposed to be a pure virtual function eventually, and it's time to start moving that way.
We're making that pure virtual in non-deprecated builds now.
2008. This has been buggy since I wrote it in 2008. And yet it took a double-refinement test on a distributed mesh plus an extra layer of dbg-mode internal consistency checks to catch the bug.
I went to double-check that this was the macro I wanted, and the behavior did match my recollection but didn't match my comment
|
Job Coverage, step Generate coverage on da90cb8 wanted to post the following: Coverage
Warnings
This comment will be updated on new commits. |
||||||||||||||||||||||||||
|
Test failures unrelated |
jwpeterson
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.
This looks good to me, but it seems like I might need to add
mesh.get_boundary_info().regenerate_id_sets();
and
mesh.cache_elem_data();
calls in some of my own code? I'm curious how you figured out where those calls were missing/needed?
Okay, "simple" might be overselling a couple commits here, but this is at least the largest collection of the most straightforward bugfixes that I could pull out of #3759.
Other than the new
InfElem::is_child_on_*unit testing, I'm not able to strip out the test coverage for these fixes; I'm afraid that depends on the refactoring in #3759, and I'm hoping to merge the refactoring last, after it's been decluttered via this and another PR.