parthenon-hpc-lab / parthenon

Parthenon AMR infrastructure
https://parthenon-hpc-lab.github.io/parthenon/
Other
112 stars 33 forks source link

Add WithFluxes to IsRefined check #1127

Closed pdmullen closed 3 months ago

pdmullen commented 3 months ago

PR Summary

The fluxes associated with a field marked Metadata::WithFluxes must undergo flux correction with mesh refinement. Parthenon cannot currently enroll custom PR ops on those fluxes if the associated field is marked Metadata::WithFluxes, but not Metadata::Independent, Metadata::FillGhost, or Metadata::ForceRemeshComm.

This PR simply adds WithFluxes to the IsRefined check to circumvent this issue. Technically, this is a bit slimy, because in the example above, the field itself does not undergo any custom-ops, only its associated fluxes, nevertheless, this appears to be the most expedient solution, and IsRefined should not actually be used when packing relevant fields needed for a particular refinement op (please check me on this one, though: https://github.com/search?q=repo%3Aparthenon-hpc-lab%2Fparthenon+isrefined&type=code). If this is too dirty, please feel free to shoot down this PR.

PR Checklist

pdmullen commented 3 months ago

So as a sanity check, I just went through and reminded myself how the bvals stuff works here. For the most part, metadata is used. the ForEachBoundary loop checks for Metadata::FillGhost and Metadata::Flux and does not use IsRefined.

IsRefined is used to help build the map from refinement functions to variables. I think this is needed regardless, so this is all fine. I would like to maybe rename IsRefined though, given we're now changing the meaning... or at least add a comment above the function explaining.

Thanks for the sanity check. I had also come to the same conclusion. I agree with your comment regarding renaming IsRefined. d7c771cd7c771c renames IsRefined -> HasRefinementOps.