Skip to content

Add call to the positivity limiter after coarsen / refine#97

Merged
andrewwinters5000 merged 17 commits intotrixi-framework:mainfrom
andrewwinters5000:amr-positive
May 8, 2025
Merged

Add call to the positivity limiter after coarsen / refine#97
andrewwinters5000 merged 17 commits intotrixi-framework:mainfrom
andrewwinters5000:amr-positive

Conversation

@andrewwinters5000
Copy link
Copy Markdown
Member

This PR re-implements max_abs_speeds for the time step calculation that uses Base.sqrt. Also, specialized refine! and coarsen! functions are implemented to ensure positivity of the water height after adaptation in the AMR callback. There were negative water heights generated (for example in the coarsening step) in wet/dry transition areas.

@codecov
Copy link
Copy Markdown

codecov bot commented May 7, 2025

Codecov Report

Attention: Patch coverage is 97.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.19%. Comparing base (464ac24) to head (a8d77a6).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/callbacks_step/amr_dg2d.jl 97.37% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
- Coverage   99.23%   99.19%   -0.04%     
==========================================
  Files          71       72       +1     
  Lines        3370     3449      +79     
==========================================
+ Hits         3344     3421      +77     
- Misses         26       28       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrewwinters5000
Copy link
Copy Markdown
Member Author

The missed lines are due to the fact that we do not run anything with MPI in the CI testing. This could be added (I think) relatively easily. It might be good to run a couple tests in parallel in TrixiSW to make sure that this capability is still available.

@andrewwinters5000
Copy link
Copy Markdown
Member Author

andrewwinters5000 commented May 7, 2025

Nevermind, there is no calc_mpi_interface_flux with nonconservative::True() implemented in Trixi. So adding such a test is outside the scope of this PR. We will have to live with the ≈0.16% drop in coverage for these six untested lines. Or we could comment them out with a note about what needs done for MPI + noncons + Trixi.

Copy link
Copy Markdown
Member

@patrickersing patrickersing left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the AMR test. Hopefully, this will also make the AMR procedure more robust.
The implementation looks good to me, I just left suggestions for some of the comments.

@patrickersing patrickersing changed the title Add call to the poisitivity limiter after coarsen / refine Add call to the positivity limiter after coarsen / refine May 8, 2025
andrewwinters5000 and others added 2 commits May 8, 2025 13:34
Co-authored-by: Patrick Ersing <114223904+patrickersing@users.noreply.github.com>
@DanielDoehring
Copy link
Copy Markdown
Member

Would it be possible to extend this to the main repository?

Copy link
Copy Markdown
Member

@patrickersing patrickersing left a comment

Choose a reason for hiding this comment

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

LGTM!

@andrewwinters5000
Copy link
Copy Markdown
Member Author

Would it be possible to extend this to the main repository?

I agree that this is something that should be done in main repo to address trixi-framework/Trixi.jl#2064.

I thought about this but I could not figure out a proper and equation agnostic way to have the call to the limiter. For shallow water I know that we always want to limit the water height and that the threshold lives in the equations such that I could hard code the limiter variable in the call

   # Apply limiter to ensure admissible solution states
    limiter_shallow_water!(u, equations.threshold_limiter, waterheight,
                           mesh, equations, dg, cache)

However, for something like compressible Euler one would limit the density and pressure but (potentially) with different threshold values. Because the AMR callback does not "know" about the stage limiter callback it is difficult to extract this information.

@andrewwinters5000 andrewwinters5000 merged commit a2d5afc into trixi-framework:main May 8, 2025
18 of 21 checks passed
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.

3 participants