cholla-hydro / cholla

A GPU-based hydro code
https://github.com/cholla-hydro/cholla/wiki
MIT License
60 stars 32 forks source link

Cholla unit system #401

Open brantr opened 5 days ago

brantr commented 5 days ago

One issue that I need to address in resolving problems with the cosmology runs and in merging in the RT is the unit system. This issue could affect others, so I thought I'd describe what I'm thinking and request discussion.

There are several of the unit systems defined in the code. Many of these units are defined in global.h. However, there are internal inconsistencies. For instance:

MYR                                  3.153600000e+13. [myr in s]
TIME_UNIT                            3.155690000e+10. [kyr in s]

Neither of these are what I expected, although TIME_UNIT is close.

Also, this:

#define MP          1.672622e-24  // mass of proton, grams
#define MH       1.67262171e-24    // Mass of hydrogen [g]

Also GN is defined in kpc^3 / M_sun / kyr^2 but there's a commented-out CGS version.

There are other unit system questions I have with the cosmology and chemistry unit systems. Some units are set inline in functions.

Here are the things I would like to pursue:

1) Refining the unit systems defined in global.h. Clearly this could impact everyone and unit tests, so this would not be taken lightly and would need input from everyone. Relabel all units that are in CGS with a _CGS suffix. Label everything unit in the kpc|km|s|Msun system and the kpc|kyr|Msun system with distinct suffixes.

2) Writing a Grid3D member function that will print the actual unit system to stdout at the beginning of a simulation, called in main(). This could be couched in a preproc def and set at compile time to print or not, but really it would be helpful. Or it could be written to a separate text output file.

3) Write functions for cosmology and chemistry where all the units for those modules are set in function members of Cosmology and Chem.

4) Write functions that print the cosmology and chemistry units when they are defined, which would be called by the grid member function.

While I listed 1-4, 2-4 is actually a tractable and small change that should impact no one. I am planning to submit a PR that addresses 2-4, but 1) probably requires discussion and so I thought I'd submit this issue. Thanks!

evaneschneider commented 4 days ago

Thanks for starting this discussion, Brant. I agree that the current unit system needs reform. Your approach for 2 - 4 seems very reasonable. Regarding 1, I think a lot of the duplicate units were added when the cosmology / chemistry code was added, so we will probably need to update test data, initial conditions, the older cooling code, etc. if the plan is to reconcile to the newer units. I am happy to help with that. The only units that should definitely stay defined are the ones labeled things like TIME_UNIT, LENTH_UNIT, etc. These have always been intended to be the internal units that Cholla uses to convert between code units and CGS, and they are currently output in the hdf5 header files and used by other analysis code (for example, the Cholla yt frontend). That said, if the actual values are off (I have no idea why kyr is defined that way instead of as 3.1536e10, for example), we should correct them. (And again, I'm happy to help with that, as needed.)

brantr commented 4 days ago

Thanks @evaneschneider .

I will work on the functions for items 2-4 and submit a PR for them.

Regarding TIME_UNIT and kyr, it's just a question of whether to pick 365 or ~365.24... or 365.25 as a "year". Note that NIST defines a lightyear with 1 day = 86400s and a Juilan century as 36525 days, which would be 3.15576e10 seconds in a kiloyear. This definition is consistent with the NIST value for a light year of 9.46073e17 cm when defining c = 29979245800 cm/s.

https://www.nist.gov/pml/special-publication-811/nist-guide-si-appendix-b-conversion-factors/nist-guide-si-appendix-b8#L