Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leftover chemistry fixes #553

Open
felker opened this issue Dec 28, 2023 · 2 comments
Open

Leftover chemistry fixes #553

felker opened this issue Dec 28, 2023 · 2 comments

Comments

@felker
Copy link
Contributor

felker commented Dec 28, 2023

I merged #496 since it was 99% complete thanks to @munan, happy new years everyone! GitHub's servers also seemed to be struggling with the long PR history and review comments, which were impossible to decipher given all the interleaving changes. I will document a few of the unaddressed comments here, which we can address now or eventually:

  • @tomidakn
    • inputs/chemistry/athinput.chem_tigress might be better excluded from the source code, since it requires TIGRESS data?
    • src/utils/interp.cpp function names like LP1D, LP2Di, etc. are not self explanatory. Use longer names. (also, the Doxygen comment says " ID arrray linear interpolation", with typos at 1D and "array")
    • src/units/units.cpp, regarding things like code_time_cgs_ defined in this folder, "It is better to minimize problem specific codes in the master branch. Technically this can be done in the input file for each problem (i.e., always "custom"). Is there any reason to have them hard coded?"
  • @felker
    • inputs/chemistry/athinput.pdr_moving and other input files have <problem> block input variables like Tw which are seemingly unused in the problem generator file. If there is a reason for still keeping them in the input file, it should be documented in a comment
@munan
Copy link
Contributor

munan commented Jan 15, 2024 via email

@felker felker mentioned this issue Apr 1, 2024
@felker
Copy link
Contributor Author

felker commented Apr 30, 2024

delaying until after the 24.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants