New AMR indicator IndicatorNodalFunction#2881
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
DanielDoehring
left a comment
There was a problem hiding this comment.
Thanks for getting this started! I added a first review, which is mostly about namings and the evaluation of the indicator at the cell center.
Furthermore, I am not sure if Positional is good naming, as this somewhat drops that you can also add time as an argument.
Included suggested style improvments Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
update Naming to indicator_function Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
changed naming Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
changed naming Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
changed naming Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2881 +/- ##
==========================================
+ Coverage 96.76% 97.07% +0.31%
==========================================
Files 610 611 +1
Lines 47500 47565 +65
==========================================
+ Hits 45960 46171 +211
+ Misses 1540 1394 -146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nging IndicatorMax with equivalent IndicatorPositional
…jl into IndicatorPositional
DanielDoehring
left a comment
There was a problem hiding this comment.
Implementationwise, this is already looking very good to me! Now we only need to cover the code & set up tests.
ranocha
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
IndicatorNodalFunction
ranocha
left a comment
There was a problem hiding this comment.
Thanks, this looks good to me.
|
The CI failure seems to come from the recent SciML updates (OrdinaryDiffEq.jl and supporting ecosystem). Running the failing test locally passes with older versions of the SciML packages, but fails after updating them. |
Co-authored-by: Daniel Doehring <doehringd2@gmail.com>
Positional Indicator and Testcases
The PR adds a Positional Indicator which allows us to define a function
rule(x,t)and use it to drive AMR.As mentioned in #2880 this is targeted for testing purposes to create highly dynamic AMR grids to test refinement, unrefinement, and mortars, especially with MPI.
As an example, I have attached a video of the
examples/p4est_2d_dgsem/elixir_navierstokes_lid_driven_cavity_amr_mortarTestcase.jlcase showcasing the capability by pulsing the refinement zone with a sin function.The testcases are envisioned to test MPI mortars for the hyperbolic parabolic system as mentioned in #2880 .
Open question
@DanielDoehring, I hope this clarifies the feature's use case.
Disclaimer
LLMs have been used to aid in the PR, though sparingly.
Funding Statement
This work has been funded by the Deutsche Forschungsgemeinschaft (DFG, German Research Foundation) – Project Number 237267381 – TRR 150.