Gas-Gas Coupling and Sparse - #136
Conversation
pdmullen
left a comment
pdmullen
left a comment
There was a problem hiding this comment.
Overall, I’m not convinced this PR makes all of the changes needed to correctly support sparse multi-species gas evolution. My preference would be to keep this PR focused on gas-gas coupling, and defer the more general sparsity work to a follow-up PR where we can address it systematically.
Once we allow multiple gas species, we also need to make explicit choices about how the rest of the physics packages interface with them. Right now, several places appear to use species 0 as the default material state, e.g. indexing into the EOS or opacity arrays with 0. Is that the intended placeholder behavior for now? If so, I think we should document and guard that limitation clearly. Otherwise, we probably need to decide whether those packages should loop over active species, construct some effective mixture state, or reject multi-species gas configurations until they are made species-aware.
Since this PR does touch sparsity, I also want to raise a broader design concern. When I usually think about sparse fields in Parthenon, I’m imagining a bulk equation with some number of constituents. In that model, you still require at least one constituent to be present because otherwise there is no bulk fluid on which to solve the hydrodynamics equations. If all constituents vanish, you are effectively trying to evolve hydrodynamics in vacuum, and a lot of assumptions can break.
This case is a little different because we are not evolving one bulk momentum and bulk energy equation; we are evolving equations for each individual gas species. Still, I’m not sure we have fully thought through what happens when all gas species deallocate on a block. Is that state permitted? If so, what should the hydro timestep, boundary exchange, reconstruction, fluxes, radiation coupling, drag, gravity, diffusion, and derived-variable fills do on that block? If not, do we need to enforce that at least one gas species remains allocated, or introduce some notion of a bulk/background fluid inside Artemis?
I think it would be useful to start that design conversation before treating sparse multi-gas evolution as supported by this PR.
| # bound but gets entrained by species 0 through drag. | ||
|
|
||
| <artemis> | ||
| problem = escape_1d |
There was a problem hiding this comment.
indentations are not consistent with style of other inputs.
| w_p = eos_d.PressureFromDensityInternalEnergy(w_d, w_s, lambda); | ||
| w_b = eos_d.BulkModulusFromDensityInternalEnergy(w_d, w_s, lambda); | ||
| w_t = eos_d.TemperatureFromDensityInternalEnergy(w_d, w_s, lambda); | ||
| w_p = eos_d(n).PressureFromDensityInternalEnergy(w_d, w_s); |
There was a problem hiding this comment.
I have some concerns already if we are truly trying to implement sparse in this changeset...
Doesn't eos_d contain all gas species EOS? So don't we need to index into eos_d via sparse_id?
If it were up to me, I'd keep this PR as just gas-gas coupling and then fully thread through sparse in a subsequent PR.
|
|
||
| // Get diffusion coefficient | ||
| Diffusion::DiffusionCoeff<DTYP, GEOM, Fluid::gas> dcoeff; | ||
| const Real mu = dcoeff.Get(dp, coords, xv, dens, sieg, eos_d(n)); |
There was a problem hiding this comment.
not sparse aware... not going to point these out anymore in this review.
| const Real &gsie = vmesh(b, gas::prim::sie(0), k, j, i); | ||
| const Real kT = eos_d.TemperatureFromDensityInternalEnergy(gdens, gsie); | ||
| const Real &gbulk = eos_d.BulkModulusFromDensityInternalEnergy(gdens, gsie); | ||
| const Real kT = eos_d(0).TemperatureFromDensityInternalEnergy(gdens, gsie); |
There was a problem hiding this comment.
are we going to see this pattern in a lot of our packages? If so, should we have a PARTHENON_REQUIRE that gas/nspecies==1 if enrolling those physics packages?
It is a big lift to propagate multifluid throughout the entire codebase so we don't need to add that now. but this kind of pattern seems to silently do wrong thing if someone is invoking gas/nspecies != 1
| auto opacity_h = packages.Get("gas")->Param<std::vector<MeanOpacity>>("opacity_h"); | ||
| auto scattering_h = | ||
| packages.Get("gas")->Param<std::vector<MeanScattering>>("scattering_h"); | ||
| packages.Add(jaybenne::Initialize(pin.get(), opacity_h[0], scattering_h[0], |
There was a problem hiding this comment.
In the grand scheme of artemis/jaybenne development, when is the right time to assign a "total" opacity which combines all fluid types and pases a FIELD rather than opacity object to jaybenne?
| const int scr_level = pin->GetOrAddInteger("gas", "scr_level", 0); | ||
| params.Add("scr_level", scr_level); | ||
|
|
||
| params.Add("radius", pin->GetOrAddInteger("gas", "radius_cgs", 0.0) * |
2 participants
Background
This adds Chapman-Cowling style collisional coupling between gas species. For this to work I've gone and done the following:
kokkos-kernelsto the submodules. This is used in the coupling solve which has to invert a matrix.I think this is ready for review now.
Description of Changes
Checklist
// This file was created in part or in whole by generative AI