-
Notifications
You must be signed in to change notification settings - Fork 23
EOS models, unit conversion, particle fixups #429
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
base: develop
Are you sure you want to change the base?
Conversation
|
Help me understand what is going on here. It seems like singularity has a ton of dependencies but I don't understand why we have the Spheral Spack package depending on Singularity's dependencies and why we are bringing them into our CMake. Neither of those things should be happening. It is possible to use a newer Spack recipe for Singularity than what is in the current version of Spack we are using, if that will help you. You might have to tweak it a little. |
It is onerous to have to include all of these, yes, but I couldn't get it to work otherwise. The dependencies are explicitly listed because the Do you have any suggestions on how to make this work in a nicer way? I agree that it would be great to get this down to a single It would also be great to be on a newer version of |
|
Did you try adding the You can get a newer version of a TPL than is in the builtin by just adding a package file in our |
kokkos is one of the required libraries for singularity. It uses it even off of the GPU. The Spiner EOS is the main one of interest here, since it gives another option for tabulated data. I can try pruning the dependencies and using |
|
Based on the singular-eos Spack package recipe, Kokkos is an optional variant. Unless this is a mistake, it implies that Singularity can work without Kokkos. By not including |
From what I remember, kokkos is an optional variant, but is required for several of the EOS options that we want. I will try to build without, but it may not work. |
|
Ok. If you determine you need it, you have to add the |
|
I have tried all sorts of configurations to get Singularity to build with fewer changes to the build files, but I am not the expert that @ldowen is with spack. Anytime I remove one of the dependencies, the I would rather not put too much work into it for now since the Singularity spack package changes a lot with the version compatible with the spack v1.0.0 release. Once that comes out, it might be worth revisiting and seeing if some of the pain points have been resolved. |
|
Have you guys started doing internal tests for this PR? Working in 1D with the gnuplot files is onerous, so I made the siloPointmeshDump compatible with 1D and made that the default (which can be reverted if anyone is actually using the gnuplot version in production). It depends on the changes to siloMeshDump in this PR. Would you prefer that I add them here or create a separate PR that depends on this one? |
|
There is an existing Spheral1dViz method that outputs the data as a table of numbers. This should be pretty friendly to all sorts of 1D plotting packages -- what's different in your version? |
|
Ah I see. Personally I don't like using Visit for 1D plots but I can understand this would be useful for folks that do. I'm happy to have this as an option for 1D viz output, but I don't think it should be the default. |
|
Another question on a different change: the RelaxNodes package added for iterateIdealH. It appears to me this duplicates the relaxNodeDistribution functionality we have in NodeGenerators, which is invoked by SpheralController optionally. We might want to rearrange how that's used a bit to fit your use case here? I'd rather not have iterateIdealH also adjust the point positions, which seems like an unexpected side-effect of converging the smoothing scale distribution. |
|
Sure, we don't need to make visit the default in 1D. The reason RelaxNodes is optionally included in the H iterations is just convenience, since iterateIdealH already includes the Voronoi evaluation and other initializations. From what I can see, prerelaxNodeDistribution doesn't call relaxNodeDistribution but instead calls an hourglass control package. The relaxNodeDistribution function doesn't seem to be tested or used anywhere except testStar2d.py, which isn't in the testing suite. It uses an old mesh object rather than the Voronoi cells. All that said, there's no reason to add RelaxNodes to the repo if you don't think it will be generally useful. It meets some of our needs for separating and differentiating points when generating them from a non-ideal distribution, but that isn't a situation anyone is likely to encounter when using the built-in generators. It seems like the easiest path forward is:
|
…hout depending directly on the dependencies

Summary
Features
Singularity EOS
Null EOS
NullEquationOfStateis the counterpart ofNullStrength.Unit conversion
PhysicalConstants. Very helpful for things like Singularity above.Subdirectories in silo
Improvements for the smoothing parameter
iterateIdealHcalculations. To use it, createrelaxation = RelaxNodes(...)and then pass it to the controller:SpheralController(initialHExtraPackages = [relaxation]). This separates nodes that are too close and relaxes nodes using the Voronoi, without which the smoothing parameter may not converge for problems with problematic initial node distributions.iterateIdealHcalculations get underway.Verbose time step control
verboseStepparameter to theIntegrator. Printing the verbose time step info every cycle makes things difficult to read. With the verbose step set to 100, it is reasonable to keep it on all the time.Bugfixes
SPHand requested Voronoi cells instead.allReduceLocto prevent the time step from being printed once per rank when they all calculate the same time step.gradientPairsmethod that is more robust for points with difficult H distributions (yes, more smoothing parameter issues). Also cleaned up thegradientinstantiation.ToDo :
RELEASE_NOTES.mdwith notable changes.