Get torus working with nuclear EoS

AstroBarker commented 1 year ago

This PR adds necessary functionality to get the torus problem working with StellarCollapse EoS.

PR Summary

I add the following functionalities:

adiabats.hpp which includes necessary functions to compute adiabats from the table given S, Ye.
- A unit test for this has been added.
In the torus pgen, I pull in the various quantities needed (adiabatic S, Ye, table bounds). The calculations for rho and u are replaced by GetStateFromEnthalpy which returns rho and u for either the ideal gas or stellar collapse EoS.

Misc change: bug found in root_find where status was not passed through correctly and it complained when the root find failed even when status was passed in.

TODO:

@brryan When you have time, can you verify that this change does not break things for your applications? (Note: You may need to update singularity-eos to its latest and checkout jhp/entropy.
I would like to unify the adiabat framework to work for the ideal gas as well, but I need to reason around this kappa.
@brryan Do you know of anything immediately obvious that I would need to change for this to work with radiation? I haven't modified any of those parts of the pgen yet.
This requires the jhp/entropy branch of singularity-eos. Once that is through I need to update the used version of singularity to latest commit. @jonahm-LANL I tried committing singularity-eos to latest but I think it did not stick. What is the way to do this?

PR Checklist

[x] Adds a test for any bugs fixed. Adds tests for new features.
[x] Format your changes by calling scripts/bash/format.sh.
[x] Explain what you did.

AstroBarker commented 1 year ago

Also the logic to make it work with IdealGas is kind of hacky.. I would like to fix this in the future by unifying the adiabat framework to work for an arbitrary EoS (without crash) but I need to better understand the kappa business first, and also this PR is already quite long..

Yurlungur commented 1 year ago

@AstroBarker looks like the singularity-eos submodule this is pinned to doesn't contain the entropy... Maybe something funny with git submodules?

Yurlungur commented 1 year ago

@AstroBarker looks like the singularity-eos submodule this is pinned to doesn't contain the entropy... Maybe something funny with git submodules?

Oh I just saw your comment above. Not sure what's going on here. I'll chat with you on slack

brryan commented 1 year ago

@AstroBarker This should work fine with the existing radiative torus setup. To do that, we take in a GRMHD solution with known density/temperature everywhere, and then in each zone treat the GRMHD pressure as the total pressure and then solve the equation P_tot = P_gas(T) + P_rad(T) for T, where T will in general be less than the initial GRMHD solution's temperature. As long as the EOS used by your PR and that gas/rad partitioning step are the same (they should be of course) this should be fine.

I will try to test for my use case soon, and at least review/approve ~tomorrow.

Yurlungur commented 1 year ago

@brryan we're currently facing a GPU bug with singularity-eos. Trying to track it down.

Yurlungur commented 1 year ago

@AstroBarker where is this as far as being able to merge it in?

AstroBarker commented 1 year ago

@AstroBarker where is this as far as being able to merge it in?

I've got some local changes that I need to finish testing and push. The changes try to use the adiabat framework for the ideal gas after converting kappa -> entropy. Letting the root find go to much lower rho, T like we talked about helps but it's still a bit fickle to get rho_rmax the same, but is closer than before.

Yurlungur commented 1 year ago

@AstroBarker where is this as far as being able to merge it in?

I've got some local changes that I need to finish testing and push. The changes try to use the adiabat framework for the ideal gas after converting kappa -> entropy. Letting the root find go to much lower rho, T like we talked about helps but it's still a bit fickle to get rho_rmax the same, but is closer than before.

How far off are they from each other now?

AstroBarker commented 1 year ago

@AstroBarker where is this as far as being able to merge it in?

I've got some local changes that I need to finish testing and push. The changes try to use the adiabat framework for the ideal gas after converting kappa -> entropy. Letting the root find go to much lower rho, T like we talked about helps but it's still a bit fickle to get rho_rmax the same, but is closer than before.

How far off are they from each other now?

Only a couple percent, so long as things are set correctly. It took some playing with e.g., how close to 0 I let the density/temp go on the adiabats and the number of samples used.

Yurlungur commented 1 year ago

@AstroBarker where is this as far as being able to merge it in?

I've got some local changes that I need to finish testing and push. The changes try to use the adiabat framework for the ideal gas after converting kappa -> entropy. Letting the root find go to much lower rho, T like we talked about helps but it's still a bit fickle to get rho_rmax the same, but is closer than before.

How far off are they from each other now?

Only a couple percent, so long as things are set correctly. It took some playing with e.g., how close to 0 I let the density/temp go on the adiabats and the number of samples used.

Hmm. That's probably fine... but since the torus is a standard test problem, it's probably useful to be able to retain the old original ideal gas setup as well.

How gross would it be to include both that machinery and a runtime flag that reverts to the old analytic formula? Not set on the eos type but something like torus/use_legacy_init or something?

AstroBarker commented 1 year ago

Have elected to keep the switches on EoS in the torus init. Made a few changes to root find tolerances. I think this is good to go. There may need to be some additional work when it is time for radiation, but that can be done at that time.

Yurlungur commented 1 year ago

Looks like tests are failing: https://github.com/lanl/phoebus/actions/runs/4622700920/jobs/8175695524?pr=159#step:5:344

AstroBarker commented 1 year ago

Thanks for the feedback @Yurlungur -- all comments should be addressed.

Yurlungur commented 1 year ago

Will merge as soon as tests pass. :+1:

lanl / phoebus