IAS-Astrophysics / athenak

Performance-portable version of the Athena++ astrophysical AMR-MHD code using Kokkos.
BSD 3-Clause "New" or "Revised" License
34 stars 24 forks source link

chakrabarti non-radiative torus option - [merged] #518

Closed jmstone closed 3 months ago

jmstone commented 1 year ago

In GitLab by @gonihalevi on Feb 28, 2023, 14:21

_Merges chakrabarti_nonrad -> master

This MR adds the option to initialize a torus with the Chakrabarti 1984 equilibrium solution, which corresponds to thinner tori than the Fishbone-Moncrief (FM) equivalent. There are new parameters for the GR torus that specify which torus to set up: "fm_torus" or "chakrabarti_torus" need to be set to true. After input from Patrick on Jim's original implementation, I've corrected the velocity initialization so the FM torus still works, removed the temperature variable since it isn't necessary for the non-radiating torus, and calculated log(h) in the same function for the two torus configurations instead of using the actual enthalpy. I have tried to make the implementations as consistent with one another as possible, and have checked that both torus initializations work. One caveat is that to avoid NaNs when taking the log of enthalpy, I've calculated log(h) for the Chakrabarti torus as the log of the difference between h at a point and h at the torus inner edge directly, rather than calculating the two logs separately and subtracting the edge value from the value at a point. This is slightly different than from the FM torus, but I made the framework identical so that it still subtracts off log(h)_edge to calculate whether we are inside the torus, but in the Chakrabarti case, its value is zero because it is already accounted for when calculating log(h). This is not super clean but it keeps the NaNs at bay and is consistent.

jmstone commented 1 year ago

In GitLab by @gonihalevi on Feb 28, 2023, 14:21

requested review from @jmstone216

jmstone commented 1 year ago

In GitLab by @gonihalevi on Feb 28, 2023, 15:06

added 3 commits

Compare with previous version

jmstone commented 1 year ago

In GitLab by @gonihalevi on Feb 28, 2023, 15:09

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @pdmullen on Mar 1, 2023, 12:09

Commented on src/pgen/gr_torus.cpp line 794

this one is my fault...

Can we switch to numerical values for variable names to be consistent with everywhere else? e.g., g_tt -> g_00. This appears in several places throughout the pgen.

jmstone commented 1 year ago

In GitLab by @pdmullen on Mar 1, 2023, 12:09

Commented on src/pgen/gr_torus.cpp line 668

also Chakrabarti

jmstone commented 1 year ago

In GitLab by @pdmullen on Mar 1, 2023, 12:10

LGTM!

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:15

Commented on src/pgen/gr_torus.cpp line 794

changed this line in version 4 of the diff

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:15

Commented on src/pgen/gr_torus.cpp line 668

changed this line in version 4 of the diff

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:15

added 1 commit

Compare with previous version

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:17

Commented on src/pgen/gr_torus.cpp line 668

added reference to Chakrabarti

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:17

Commented on src/pgen/gr_torus.cpp line 794

yep, changed all instances.

jmstone commented 1 year ago

In GitLab by @gonihalevi on Mar 1, 2023, 14:17

resolved all threads

jmstone commented 1 year ago

In GitLab by @pdmullen on Mar 1, 2023, 17:14

approved this merge request

jmstone commented 1 year ago

In GitLab by @jmstone216 on Mar 2, 2023, 10:49

mentioned in commit ebb894a74a84cfec712d4b587e9938ecc23b2ac2