Skip to content

Make is_prepared() more fine-grained #3759

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.

Sign up for GitHub

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

Jump to bottom
Open
roystgnr wants to merge 16 commits into
base: devel
Choose a base branch
from
Open

Make is_prepared() more fine-grained #3759

roystgnr wants to merge 16 commits into from
+501 −73

Conversation

@roystgnr
Copy link
Member

@roystgnr roystgnr commented Jan 4, 2024

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.

@roystgnr roystgnr self-assigned this Jan 4, 2024
@lindsayad lindsayad mentioned this pull request Jan 4, 2024
Merged
@lindsayad
Copy link
Member

lindsayad commented Jan 4, 2024

Does "do not merge" also mean "do not review"?

@moosebuild
Copy link

moosebuild commented Jan 4, 2024

Job Coverage on 7c8c1d1 wanted to post the following:

Coverage

ffa3a3 #3759 7c8c1d
Total Total +/- New
Rate 62.35% 62.36% +0.02% 98.67%
Hits 68275 68328 +53 74
Misses 41235 41236 +1 1

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@roystgnr
Copy link
Member Author

roystgnr commented Jan 4, 2024

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
lindsayad reviewed Jan 5, 2024
View reviewed changes
Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just looking at the header this seems like a great foundation to me

@roystgnr roystgnr force-pushed the mesh_preparation_finer branch from 7c8c1d1 to fc6ee94 Compare June 21, 2024 15:16
roystgnr added 15 commits December 5, 2025 14:35
@roystgnr
Make is_prepared more fine-grained
aa69f30
@roystgnr
Assert our cache is still good
ca50100
@roystgnr
Make PerfLog name accurate
b2b95e0
@roystgnr
More detailed preparation comments
9b3e41c
@roystgnr
Test partially-prepared status validity too
95432e8
@roystgnr
Update comments
de39e08
@roystgnr
Don't try to use an unprepared mesh in testing
1814a35 
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.
@roystgnr
cache_elem_data() after modifying mesh subdomains
64b5b0f 
Otherwise the mesh still thinks its fully prepared even when it isn't
and we can't assert valid_is_prepared() on it.
@roystgnr
Prepare mesh_copy before smoother solves on it
cc28c22 
We need things like neighbor pointers, and in general we'd like to
assert that we're never assembling or solving on unprepared meshes.
@roystgnr
Assert that we only assemble on prepared meshes
f29a462 
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"!.
@roystgnr
MeshModification shouldn't leave invalid caches
a4fa81c 
We can rebuild point locators or element caches, but we don't want to
use an already-built invalid cache by accident.
@roystgnr
Assign _children_on_boundary in BoundaryInfo =
d19bcb2
@roystgnr
Clarify prepare_for_use() doxygen
97725fe
@roystgnr
Get interior parent pointers *right* when copying
aa107d4 
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.
@roystgnr
More testing of preparation() validity in dbg mode
e39fc75
@roystgnr roystgnr force-pushed the mesh_preparation_finer branch from fc6ee94 to e39fc75 Compare December 5, 2025 21:17
@roystgnr
Copy link
Member Author

roystgnr commented Dec 5, 2025

This might actually be in shape to merge soon. My remaining concerns:

  1. The additional internal testing I've added might trigger dbg and/or devel failures downstream, in tests of code that doesn't properly prepare a mesh before solving (so it'll fail the new assertions) but either doesn't rely on the unprepared data or just compares to a gold solution that mis-relied on it (so it's not already failing tests). I'll add every recipe I think might be affected to the CI here.

  2. The additional internal testing I've added might be too expensive in dbg mode, where it tests preparation validity before every assembly. I think we can afford that (prepare_for_use() is much cheaper if we don't repartition, which we don't when checking validity). If we see CI timeouts then I'll try moving my testing from assembly methods to solve() methods. If we still see timeouts after that then I'll remove the intensive testing entirely for now.

@roystgnr roystgnr mentioned this pull request Dec 5, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lindsayad lindsayad lindsayad left review comments

Assignees

@roystgnr roystgnr

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@roystgnr @lindsayad @moosebuild