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
jwpeterson
left a comment
There was a problem hiding this comment.
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?
|
Forcing downstream users to change their mesh generation code right away would probably be a deal-breaker, right? But IMHO there are two situations we should disallow in the long-term: leaving a mesh in a state where it thinks it's prepared (i.e. Right now (in #3759) if you fail such an assertion, you get a message with some explanation of which data doesn't match between your mesh and a properly-prepared copy of your mesh ... but I should probably also make the explanation more fine-grained than it is now; I lumped every failure of |
3 participants
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.