Custom refinement ops propagated to fluxes

[brryan]

PR Summary

Custom refinement operations enrolled in variable metadata were not being propagated to flux fields if WithFluxes was active. This PR addresses that.

Note that in this model the refinement operations will need to be able to distinguish between the field and the flux field, presumably by branching based on the TopologicalElement.

PR Checklist

• Code passes cpplint
• New features are documented.
• Adds a test for any bugs fixed. Adds tests for new features.
• Code is formatted
• Changes are summarized in CHANGELOG.md
• Change is breaking (API, behavior, ...)
  • Change is additionally added to CHANGELOG.md in the breaking section
  • PR is marked as breaking
  • Short summary API changes at the top of the PR (plus optionally with an automated update/fix script)
• CI has been triggered on Darwin for performance regression tests.
• Docs build
• (@lanl.gov employees) Update copyright on changed files

[Yurlungur]

Nice. Glad this worked. Do we want to propogate the exact same refinement functions? Or do we want to create a second set of refinement functions for fluxes? I guess the topological element is always different, so they're always distinguishable?

[brryan]

Nice. Glad this worked. Do we want to propogate the exact same refinement functions? Or do we want to create a second set of refinement functions for fluxes? I guess the topological element is always different, so they're always distinguishable?

Yeah the approach we're taking is to have fluxes get the same refinement functions. As you say they are always distinguishable by topological element, and that is a template parameter, so I don't think there is any cost to doing things anyway, and it is compact. If this seems like a bad design choice though lets discuss that here before merge.

[pdmullen]

This PR fixes a breaking change for downstream codes needing custom flux correction routines, therefore, we should likely get it in sooner rather than later...

One comment for future consideration... If we have a field that is NOT Metadata::Independent, NOT Metadata::ForceRemeshComm but it does have Metadata::WithFluxes, it seems strange that we have to enroll all custom PR ops for that field even though the only thing it needs is flux correction.

Actually, is Metadata::WithFluxes enough to trigger IsRefined so that the flux correction is even enrolled?

[pdmullen]

Actually, is Metadata::WithFluxes enough to trigger IsRefined so that the flux correction is even enrolled?

the answer is yes:

parthenon/src/interface/metadata.hpp

Line 451 in ff6a943

IsSet(GMGProlongate) || IsSet(GMGRestrict) || IsSet(Flux));

[lroberts36]

LGTM. I agree that this is a general solution since the restriction operation can choose the method based on the topological element, although in some cases it might be cleaner to allow specifying a separate restriction operation for the flux field.

[Yurlungur]

Nice. Glad this worked. Do we want to propogate the exact same refinement functions? Or do we want to create a second set of refinement functions for fluxes? I guess the topological element is always different, so they're always distinguishable?

Yeah the approach we're taking is to have fluxes get the same refinement functions. As you say they are always distinguishable by topological element, and that is a template parameter, so I don't think there is any cost to doing things anyway, and it is compact. If this seems like a bad design choice though lets discuss that here before merge.

Nah it's fine. I think as far as user intent goes, maybe a separate function would be clearer. But as far as implementation "cleanliness" it's unclear what the optimal choice is. There's trade-offs. Go ahead and merge this.