Add more integrity checks for tetrahedralization#4408
Add more integrity checks for tetrahedralization#4408roystgnr wants to merge 9 commits intolibMesh:develfrom
Conversation
|
I'm putting this up to make sure it doesn't trigger any false positives in downstream CI and to give @GiudGiud a chance to play with it right away, but leaving it as "Draft" until I find time to add some unit tests for it. |
roystgnr
force-pushed
the
more_hull_integrity_checks
branch
2 times, most recently
from
487d99e to
36864cd
Compare
|
@GiudGiud reports that this finally was enough to catch the mixed orientation in his input. I'm going to tack on some commits to fix those problems, not just to test the tests for them, but this should be ready to review/merge at any time; I could add later work to a different PR instead. |
An integer error code was kind of lazy of us, and using a set lets us cleanly fix errors rather than just report them.
This might be useful in letting application code recover from such errors, and it's definitely useful in debugging unit tests where I've screwed something up.
roystgnr
force-pushed
the
more_hull_integrity_checks
branch
from
36864cd to
55a5fef
Compare
NetGen is only happy with oriented manifolds in one orientation
|
Test coverage is good. That failure in the distributed sweeps wasn't a distributed-mesh failure, it was a general bug triggered by I think we're solid now, though. |
| Elem * elem = this->_mesh.elem_ptr(id); | ||
| const Point e01 = elem->point(1) - elem->point(0); | ||
| const Point e02 = elem->point(2) - elem->point(0); | ||
| const Point normal = e01.cross(e02).unit(); |
There was a problem hiding this comment.
could detect a sliver / 0 volume element there ?
There was a problem hiding this comment.
actually better earlier in the first call to check_hull_integrity
| if (result.count(BAD_NEIGHBOR_LINKS)) | ||
| { | ||
| err_msg << "At least one triangle neighbor with inconsistent node and neighbor links was found in the input boundary mesh." << std::endl; | ||
| } |
There was a problem hiding this comment.
This is actually a little hard to find once we get (only) that message.
Can we output the issues as they are found?
There was a problem hiding this comment.
even if libmesh is able to fix the issues, it's still good to fix the input mesh instead
There was a problem hiding this comment.
IMHO that one in particular we shouldn't; it's something that might be automatically fixed up, if the cause was just a poorly prepared input.
For the ones that are definitely unfixable input problems ... I don't want to output anything by default, honestly, but I'll add a verbose option.
include/mesh/mesh_tet_interface.h
Outdated
| MISSING_BACKLINK = 4, // an element neighbor isn't linked back to it | ||
| BAD_NEIGHBOR_NODES = 5, // an element neighbor isn't linked to expected nodes | ||
| NON_ORIENTED = 6, // an element neighbor has inconsistent orientation | ||
| BAD_NEIGHBOR_LINKS = 7 // an element neighbor has other inconsistent links |
There was a problem hiding this comment.
are these other intgrity issues covered by unit tests?
There was a problem hiding this comment.
NON_ORIENTED is covered; I figured that was the urgent one.
It's tricky to get the others to trigger realistically. My first thought was to refine an element, but that just gets caught by the libmesh_not_implemented_msg in MeshModification::all_tri().
| if (result==3) | ||
| err_msg << "The input Mesh was empty!" << std::endl; | ||
| finished_elements.insert(elem_id); | ||
| frontier_elements.erase(elem_id); |
There was a problem hiding this comment.
this only works if we have a single connected component. Do we have a check for that integrity issue?
(not asking for it now)
There was a problem hiding this comment.
this only works if we have a single connected component.
Nah - unless the mesh is self-intersecting (and that's something I'd love to check for, but I don't know how to do it in O(N)), best_elem will have been part of the external component, so the external component will end up properly oriented.
Do we have a check for that integrity issue?
No... which is partly by design. If you supply an input mesh which defines a boundary with multiple connected components, we take the outermost as the actual boundary, so you don't have to worry about "filling in" any holes.
That's a less useful feature when the input is surface rather than volume elements, I admit, but we keep the same behavior there out of laziness consistency.
| * issues as they're found. Subclasses may add other output at | ||
| * other verbosity levels. | ||
| */ | ||
| void set_verbosity(unsigned int v); |
You'd think that adding one line of do-nothing test coverage is pointless, but at least it would have caught that embarrassing omitted definition as a linker error.
3 participants
Hopefully this will catch whatever's going wrong on Guillaume's test case; I'm terrified of the possibility of having to diagnose a Netgen bug.