-
Notifications
You must be signed in to change notification settings - Fork 297
Adaptivity improvements #4184
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
Adaptivity improvements #4184
Conversation
1db4795
to
b57a129
Compare
458dc7b
to
e9bb873
Compare
e9bb873
to
c4e48be
Compare
Job Coverage, step Generate coverage on 7d2f546 wanted to post the following: CoverageInconsistent report tags were found between the head and base reports. Inconsistent tags: This comment will be updated on new commits. |
MetaPhysicL failure again 😦 |
3d07782
to
24e132a
Compare
This ensures that if there is no mesh refinement before another call to project vectors that vector projection will work Refs attempt to `disable_refine_in_reinit` in idaholab/moose#30742
Because we do not use the SC dof map in any way for solution projection. And static condensation doesn't even make much sense without matrices
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.
I think I'm happy with this, though I'd love for @jwpeterson to take a look too.
std::unordered_map<processor_id_type, std::unordered_set<dof_id_type>> _nonlocal_uncondensed_dofs; | ||
|
||
/// Used for temporary storage of uncondensed degrees of freedom active on an element | ||
std::vector<dof_id_type> _elem_uncondensed_dofs; |
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.
Likewise for these two. Though I admit we're now up to three more arguments, and my suggestion is getting less compelling, it just feels confusing to have temporary storage saved as members.
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.
So you would prefer this?
void StaticCondensationDofMap::add_uncondensed_dof(
const dof_id_type full_dof_number,
const bool involved_in_constraints,
std::unordered_map<dof_id_type, dof_id_type> & uncondensed_global_to_local_map,
std::unordered_set<dof_id_type> & local_uncondensed_dofs_set,
std::unordered_map<processor_id_type, std::unordered_set<dof_id_type>> &
nonlocal_uncondensed_dofs,
std::vector<dof_id_type> & elem_uncondensed_dofs,
dof_id_type & uncondensed_local_dof_number,
std::unordered_set<dof_id_type> constraint_dofs)
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.
c2bd8d6 uses local variables
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.
Yeah, honestly. It feels like that "scratch space" could be members of a nested class, even?
I wouldn't go that far now, though. If this was a public API I'd be anxious over getting the best design before we exposed it to users and had to support it, but with a private API we can redo at any time I'm just expressing mild preferences.
We can merge without kicking that test - MetaPhysicL failure again. I'm so baffled by that one. |
c2bd8d6
to
7d2f546
Compare
I force pushed the last commit just with the blank line removals that @jwpeterson pointed out. I'll merge if/once this passes CI |
call to project vectors that vector projection will work. Refs Don't do unnecessary refine in reinit idaholab/moose#30742