Entropy #78
Entropy #78
Conversation
AsterX: Update local branch with most recent upstream main
AsterX/src/con2prim.cxx
Outdated
| // Construct Entropy c2p object: | ||
| c2p_1DEntropy c2p_Ent(eos_th, atmo, max_iter, c2p_tol, rho_strict, | ||
| vw_lim, B_lim, Ye_lenient, cons_error_limit, use_z, | ||
| alp_thresh, rho_BH, eps_BH, vwlim_BH); |
There was a problem hiding this comment.
Just a stylistic comment to pass all the limiting parameters together:
For eg:
c2p_1DEntropy c2p_Ent(eos_th, atmo, max_iter, c2p_tol, alp_thresh, cons_error_limit,
rho_strict, vw_lim, B_lim, rho_BH, eps_BH, vwlim_BH, Ye_lenient, use_z);
Feel free to remove rho_strict all together as well.
There was a problem hiding this comment.
I can change the interface but what do you think about keeping a separate parameter for the maximum density? E.g. there might be cases where the upper limit of the EOSX maximum density can exceed the density limit applied in the C2P
There was a problem hiding this comment.
Good point. In my opinion, an extra parameter may not be necessary. Thinking of two applications:
(i) in BNS, rho_max should be ideally set to the max in the EOS table. In case of BH remnants, rho_BH would act as the limiting parameter inside the AH.
(ii) in single/double BH case, anything outside the AH can be capped to rho_max, if required. The important parameter in this case would be rho_BH.
Could you specify a case where you expect the upper limit of the EOSX maximum density could exceed the density limit applied in the C2P? If yes, how this parameter would function? Thanks!
There was a problem hiding this comment.
I see, yes I would agree that only the BH interior might be a region where this upper bound is relevant and would be different from the eos maximum. But we have that already covered by rho_BH. So, ok I can remove rho_strict then.
AsterX/src/con2prim.cxx
Outdated
| // velz(p.I), Bvecx(p.I), Bvecy(p.I), Bvecz(p.I), | ||
| Avec_x(p.I), Avec_y(p.I), Avec_z(p.I)); | ||
| } | ||
| if ( (alp(p.I) < alp_thresh) && ( (pv_seeds.rho > rho_BH) || (pv_seeds.eps > eps_BH) ) ) { |
There was a problem hiding this comment.
In case of C2P failure within the AH, I would always apply the bh_interior_fail fix instead of setting the point to atmo. The latter can create large gradients in the fluid.
I would recommend using just: if(alp(p.I) < alp_thresh)
There was a problem hiding this comment.
The check on rho and eps have been kept for consistency with the main branch but I can remove them
|
|
||
| // assert(0); | ||
| if ( (alp(p.I) < alp_thresh) && ( (pv_seeds.rho > rho_BH) || (pv_seeds.eps > eps_BH) ) ) { |
Con2PrimFactory/param.ccl
Outdated
| @@ -52,7 +52,8 @@ CCTK_REAL vw_lim "Upper limit for the value of velocity * lorentz_factor " STEER | |||
| 0: :: "Must be positive" | |||
| } 10.0 | |||
|
|
|||
| CCTK_REAL rho_strict "Density above which RePrimAnd c2p corrections are forbidden" STEERABLE=ALWAYS | |||
| # TODO: Change name | |||
| CCTK_REAL rho_strict "Density ceiling after C2P" STEERABLE=ALWAYS | |||
There was a problem hiding this comment.
Recommend to use "rho_max", and remove this
| c2p_Noble.bh_interior_fail(eos_th,pv,cv,glo); | ||
| } else { | ||
| // set to atmo | ||
| cv.dBvec(0) = dBx(p.I); |
There was a problem hiding this comment.
I would set to atmo only when "set_atmo" is set to true for both C2Ps. For eg, if there are NaNs in magnetic field or metric determinant is negative, setting the point to atmosphere would not work
There was a problem hiding this comment.
So, in these cases you would like to abort the simulation?
There was a problem hiding this comment.
Yes, that is correct. The aforementioned cases would mean that something is going really wrong in the simulation. And even when setting the point to atmo, the magnetic field and metric values are not touched, and will continue to give NaNs
There was a problem hiding this comment.
Ok, but I think this would require a bigger change in the code and it would be good to have a separate PR for that
There was a problem hiding this comment.
As "set_atmo" is currently not used in this way, I think
|
|
||
| #Initial conditions for entropy | ||
|
|
||
| schedule SetEntropy IN HydroBaseX_PostInitial |
There was a problem hiding this comment.
Since this function sets initial data for the HydroBaseX variable "entropy", I would recommend scheduling within the group " HydroBaseX_InitialData"
There was a problem hiding this comment.
I just had a quick look again at this point. The entropy requires valid pressures, densities etc. So, it should be executed after a thorn which is setting these quantities. That's comparable to the ID for the vector potential in the TOV or BNS case. As it stands now, I can't really schedule it in HydroBaseX_InitialData in full generality as it should always be run after something that sets rho, press, eps ... (if I don't introduce new schedule bins)
So, if there is no obvious problem with keeping it in HydroBaseX_PostInitial, we could keep it that way. Alternatively, we could do something similar as is done for the vector potential and use ODESolvers_Initial, but I am not sure if there is an advantage
…out explicit checks
No description provided.