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

New ASPH H update algorithm #302

Open
wants to merge 182 commits into
base: develop
Choose a base branch
from
Open

New ASPH H update algorithm #302

wants to merge 182 commits into from

Conversation

jmikeowen
Copy link
Collaborator

@jmikeowen jmikeowen commented Aug 22, 2024

Summary

  • This PR is a feature updating how we update the smoothing scales
  • It does the following:
    • Introduces a new method for updating the ASPH ideal H
    • Splits the H update algorithms into their own physics packages, removing them (and their needed data) from all hydro packages.
    • Updates how the Voronoi-like geometry is computed, making it its own service physics package akin to how the RK corrections package is handled.
    • Introduces a new experimental general hourglass control for all hydro algorithms.
    • Cleans up all Clang compiler warnings, and sets the Clang CI jobs to use Warnings as Errors to keep those builds warning free.

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#)
  • LLNLSpheral PR has passed all tests.

scale udpate rules.  Also a few minor fixes for generating ratio
spheres in 2D.
for looking up nperh information for ideal H estimation.  Preparing
for adding new interpolators based on the second moment for our new
ASPH approach.
applying limits to the allowed nperh lookup range to ensure there's
something to sample from.  Brody's TestIntegrator function is failing
now for reasons I don't understand, so leaving that alone 'til I
figure out why.
zeroth, first, and 2 second moments be passed in.  Also updated all
the hydros that call these methods to compute those moments (untested still).
functionality into the ordinary ASPH algorithm object now.
our current second moment does not reliably work
SpheralController takes care of this normally, but in some testing
situations like this we have to get manual.
a lot more polyhedron tests with the new SPH algorithm constructing
the Voronoi constantly.
and creating a special H time advance policy.  Note when using these
options the ASPH idealH does not require the Voronoi (big time savings).
…on in the

non-radial direction to account for changing r, and forced special
mode for iterateH which switches to fixShape for the iteration.
Comment on lines +394 to +396
for (auto* packagePtr: package.preSubPackages()) this->appendPhysicsPackage(*packagePtr);
mPhysicsPackages.push_back(&package);
for (auto* packagePtr: package.postSubPackages()) this->appendPhysicsPackage(*packagePtr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the pre/postSubPackages are only used for the smoothing scale at the moment. A few questions:

  • Are there other examples we should consider of things that could become subpackages?
  • Is there a way to do this without the extra complexity of subpackages? If we were missing an appendBoundary instance for one of the subpackages, for instance, that could be difficult to test and find. Although the RK and Voronoi packages are is added directly to the integrator without the extra subpackage indirection, those aren't specific to the hydro package, so maybe this extra machinery makes sense.
  • Do we need to be worried about the case where multiple packages include the same subpackage? If so, we may want to add a check for if (!havePhysicsPackage(subPackage)). It seems like the use case is that a subpackage will never be tied to a different package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those are good questions. I added these subpackages as a workaround so we could still maintain the user script level ideas of SPH/ASPH, FSISPH/AFSISPH, etc. I don't know of a better way to maintain that sort of script level backwards compatibility, and I think those concepts of the hydro including the H evolution makes sense from a user perspective.

Another example of something that might become a subpackage is the artificial viscosities. We could abstract those out of the hydro packages as well, particularly if we add the idea of pair-wise centering data type.

At the moment I don't forsee having subpackages that might be defined by multiple packages, but we can easily check for that since the way the work is the SpheralController builds the ultimate package list by adding in the subpackages from each package.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we also need the self interaction block in the CompatibleDifferenceSpecificThermalEnergyPolicy.cc for FSISPH, MFM, MFV and GSPH to conserve energy properly w/ the hour glass acceleration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current SubPointPressureHourglassControl does not create self-interactions (though at one stage it did). However, you are right that for consistency between these various compatible energy update policies we should handle the possible existence of self-interaction terms similarly. Really we need to look at merging these all into one policy, but for now I'll introduce the same self-interaction check in the CompatibleDifferenceSpecificThermalEnergyPolicy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh shoot, there is a problem. That policy also needs the pairDepsDt array to be properly sized, which it will not be (I don't think) if we're allowing self-interactions. Since there are no main-line algorithms that currently have self-interactions I say we leave this as is. We'll have to revisit this though if we for instance introduce an RZ version of any of those hydros (FSI, MFM, MFV, GSPH) since those do have self-interaction terms.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the applyFieldListGhostBoundary() methods are applied twice here: once in the postStateUpdate method of PSPH then again when it returns true to the integrator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch here -- yes, the new ability to return True and have the boundary conditions applied across all packages once is intended to replace this sort of scattered patchwork of such things like here in the PSPH::postStateUpdate. I'll remove these redundant BC applications.

@jmpearl
Copy link
Collaborator

jmpearl commented Oct 17, 2024

All the edits to FSISPH, MFM, MFV, and GSPH look good to me. I didn't get through all the files but I gave most of the other hydros a quick eye too.

jmpearl
jmpearl previously approved these changes Oct 17, 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