Make is_prepared() more fine-grained #3759
Conversation
|
Does "do not merge" also mean "do not review"? |
It's already bad enough to need fixing, huh? ;-) Nah, please jump in as you see fit. I've got more I'm wanting to add here, but if there's flaws in the foundation then it's probably better to fix them before I put more layers on top. |
lindsayad
left a comment
lindsayad
left a comment
There was a problem hiding this comment.
Just looking at the header this seems like a great foundation to me
roystgnr
force-pushed
the
mesh_preparation_finer
branch
from
7c8c1d1 to
fc6ee94
Compare
roystgnr
force-pushed
the
mesh_preparation_finer
branch
from
fc6ee94 to
e39fc75
Compare
|
This might actually be in shape to merge soon. My remaining concerns:
|
|
Job Coverage, step Generate coverage on 7143591 wanted to post the following: Coverage
Warnings
This comment will be updated on new commits. |
||||||||||||||||||||||||||
roystgnr
force-pushed
the
mesh_preparation_finer
branch
2 times, most recently
from
cc7fd1a to
9e6f0db
Compare
We need things like neighbor pointers, and in general we'd like to assert that we're never assembling or solving on unprepared meshes.
We can rebuild point locators or element caches, but we don't want to use an already-built invalid cache by accident.
It's quite annoying to me to have to hunt down *why* a valid_is_prepared() call fails, which means it's got to be utterly intractable to too many users. So, instead of just asserting the final boolean results of an operator==, let's provide some way to get assertion failures that deliver more information about what wasn't equal.
This should give a more informative assertion failure message when needed.
When NDEBUG isn't defined, these should be no-ops, not unavailable, so we don't have to wrap them in ifdefs elsewhere.
This should be a little less cryptic, anyway.
We're trying to allow meshes to keep track of "half-prepared" status, for the sake of efficiency in large chains or webs of mesh generators, but at the very least user assembly kernels should be able to rely on a mesh being prepared for use - they're what we mean by "use"!.
This should be much better for debugging cases where the assert fails.
This is actually a big behavior change, considering it led to failed assertions in a half dozen of our own examples. Let's give users some time to get up to speed.
(at least in non-deprecated builds)
We can have those on a ghosted boundary element
If we're skipping partitioning, we shouldn't mark ourselves as partitioned if we're not, and we shouldn't expect that we're marked as partitioned.
I haven't actually seen this fail yet but it does seem like the sort of thing we should be checking.
Figuring out neighbor pointers in distributed infinite element generation is hard enough that I want to just make find_neighbors do most of the work, but it'll be preliminary work that we can't safely assert parallel consistency on.
We never do parallel sweeps with infinite elements enabled in CI, but I did one and after a little work it passes.
Otherwise the nonlinear solver can fail to converge, depending on parallel n_processors() and partitioning
We're going to need an extra find_neighbors() call after mesh redistribution to handle a corner case that unpacking neighbor information doesn't, so let's make that call as cheap as possible.
There's a corner case where a processor can receive 2 ancestor elements that are neighbors, but from source processors each of which only sees one of the two semilocally. This is necessary to fix that.
roystgnr
force-pushed
the
mesh_preparation_finer
branch
from
7143591 to
6febe53
Compare
3 participants
I'm still trying to figure out how to make this easy to use, but I'd like to at least test that what I've got so far still passes CI.