Skip to content
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

Improve ElementSubdomainModifier moving boundary and reinitialization #27965

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

Wendy-Ji
Copy link
Contributor

@Wendy-Ji Wendy-Ji commented Jun 21, 2024

This pull request addresses several problems with ElementSubdomainModifier. The biggest changes in this PR are:

  • The creation of the ElementSubdomainModifierBase to contain all the code for changing subdomain, modifying sidesets/nodesets, and reinitialization (ElementSubdomainModifier now only determines which elements are changing subdomain, and their previous and new subdomain IDs)
  • Greater control over the changing sidesets/nodesets using the moving_boundaries and moving_boundary_subdomain_pairs parameters (moving_boundary_name and complement_moving_boundary_name are replaced by these)
  • Control over which changed elements are reinitialized using the reinitialize_subdomains and old_subdomain_reinitialized parameters (apply_initial_conditions is replaced by these)

This fixes:

  • The moving_boundary_name was not updating correctly and the complement_moving_boundary_name was being used instead. This redundancy has been removed, and the ability to choose which 'side' of the moving boundary to use is still retained
  • An adaptivity file (*-s0*.e) file was being output at every time step even where there were no subdomain changes. meshChanged() is now only called when at least one element changes its subdomain.
  • Segmentation fault for particular scenarios when running in parallel has been fixed

This adds:

  • moving_boundary_subdomain_pairs gives greater control over the updates of the boundary. This is based on which pairs of subdomains elements are between (only applies to elements that change subdomain ID). External element sides can now also be added to the boundary.
  • Reinitialization of variables and auxvariables was previously all or nothing for elements that changed subdomains. Now user can control whether reinitialization occurs depending on the old and new subdomain IDs
  • Reinitialization of material properties always occurred previously. Now they reinitialize on the same elements as the variables and auxvariables do

refs #25736

@hugary1995 Continues some of the work from #26254

@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch from 0059e27 to a74f4cf Compare June 28, 2024 16:39
@Wendy-Ji Wendy-Ji changed the title Improve Moving Boundary in ElementSubdomainModifier Improve ElementSubdomainModifier Jul 18, 2024
@Wendy-Ji Wendy-Ji changed the title Improve ElementSubdomainModifier Improve ElementSubdomainModifier moving boundary and reinitialization Jul 18, 2024
@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch 2 times, most recently from f493768 to b151191 Compare July 18, 2024 22:09
@moosebuild
Copy link
Contributor

moosebuild commented Jul 22, 2024

Job Documentation on 352a2d3 wanted to post the following:

View the site here

This comment will be updated on new commits.

@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch 2 times, most recently from 9127e93 to 80fdbc1 Compare July 25, 2024 21:05
@moosebuild
Copy link
Contributor

moosebuild commented Jul 26, 2024

Job Coverage on 352a2d3 wanted to post the following:

Framework coverage

0ef472 #27965 352a2d
Total Total +/- New
Rate 85.03% 85.05% +0.02% 96.32%
Hits 105219 105254 +35 314
Misses 18526 18506 -20 12

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@Wendy-Ji
Copy link
Contributor Author

Hello, this PR will cause some errors when testing malamute and racoon as the parameters have been changed for CoupledVarThresholdElementSubdomainModifier. I have made PRs to patch these. Thank you.

idaholab/malamute#160
hugary1995/raccoon#171

@Wendy-Ji Wendy-Ji marked this pull request as ready for review July 26, 2024 20:34
@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch from 80fdbc1 to 5795085 Compare July 29, 2024 20:05
@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch from 5795085 to 8858167 Compare August 19, 2024 20:18
@hugary1995 hugary1995 requested a review from dewenyushu August 20, 2024 14:27
@hugary1995 hugary1995 force-pushed the esm_moving_boundary branch from 2abf514 to 3ff4e65 Compare August 21, 2024 13:30
@hugary1995
Copy link
Contributor

#28314 resulted in some conflicts in this PR. I resolved the conflicts (hopefully correctly) and force pushed to your branch. @Wendy-Ji let me know if I missed anything. I'm still reviewing this PR.

Copy link
Contributor

@hugary1995 hugary1995 left a comment

Choose a reason for hiding this comment

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

I have some really minor comments. There are some files that need to be restored -- I will take care of them.

modules/stochastic_tools/examples/paper/results/weak.tex Outdated Show resolved Hide resolved
python/MooseDocs/test/gold/latex/extensions/include.tex Outdated Show resolved Hide resolved
python/MooseDocs/test/gold/latex/extensions/navigation.tex Outdated Show resolved Hide resolved
python/MooseDocs/test/gold/latex/extensions/plotly.tex Outdated Show resolved Hide resolved
python/MooseDocs/test/gold/latex/extensions/style.tex Outdated Show resolved Hide resolved
@hugary1995
Copy link
Contributor

Since I am not an independent reviewer as I authored part of ElementSubdomainModifierBase.h/.C, we will need at least one approval from either @dewenyushu or @GiudGiud .

@hugary1995 hugary1995 requested a review from GiudGiud August 21, 2024 15:23
Copy link
Contributor

@dewenyushu dewenyushu left a comment

Choose a reason for hiding this comment

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

Excited to see this code improvement. Very well coded and documented. I just have a few comments.

@Wendy-Ji Wendy-Ji requested a review from roystgnr as a code owner August 21, 2024 22:10
@GiudGiud
Copy link
Contributor

Will be tomorrow, got too busy

- moving_boundaries and moving_boundary_subdomain_pairs for sideset/nodeset modification
- reinitialize_subdomains and old_subdomain_reinitialized to control reinitialization of changed elements

refs idaholab#25736
@Wendy-Ji Wendy-Ji force-pushed the esm_moving_boundary branch from 23e4ab4 to 1d806e8 Compare August 28, 2024 15:15
Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

looks good

@GiudGiud GiudGiud force-pushed the esm_moving_boundary branch 3 times, most recently from 00f02ec to c29e899 Compare August 29, 2024 19:57
@GiudGiud GiudGiud force-pushed the esm_moving_boundary branch from c29e899 to 1504692 Compare August 29, 2024 20:02
@idaholab idaholab deleted a comment from moosebuild Aug 29, 2024
- more docstring
- modified doco
- deprecate old boundary parameters
@GiudGiud GiudGiud force-pushed the esm_moving_boundary branch from 1504692 to 3e645a2 Compare August 29, 2024 21:54
large_media Show resolved Hide resolved
@idaholab idaholab deleted a comment from moosebuild Sep 6, 2024
@GiudGiud GiudGiud merged commit 2ad07ee into idaholab:next Sep 12, 2024
51 of 52 checks passed
@GiudGiud
Copy link
Contributor

@Wendy-Ji if you could update your malamute patch please. If you don't have time please just give me write access and I'll finish it up

@Wendy-Ji
Copy link
Contributor Author

@Wendy-Ji if you could update your malamute patch please. If you don't have time please just give me write access and I'll finish it up

The PR is still under review, but I think everything in the patch is up to date now. I've given you write access in case there's something else I've missed

jmeier added a commit to jmeier/moose-codeplugin-opalinus that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants