Skip to content

[newchem-cpp] Cleanup initialization of kcol_rate_tables#413

Merged
brittonsmith merged 15 commits into
grackle-project:newchem-cppfrom
mabruzzo:cleanup-kcol_rate_tables_init
Nov 13, 2025
Merged

[newchem-cpp] Cleanup initialization of kcol_rate_tables#413
brittonsmith merged 15 commits into
grackle-project:newchem-cppfrom
mabruzzo:cleanup-kcol_rate_tables_init

Conversation

@mabruzzo

@mabruzzo mabruzzo commented Sep 6, 2025

Copy link
Copy Markdown
Collaborator

This PR must be reviewed after #411 is merged


Overview

At a high-level this PR:

  • deletes initialize_metal_chemistry_rates.cpp
  • mutates initialize_rates.cpp
  • creates init_misc_species_cool_rates.cpp
  • creates collisional_rate_props.cpp

In more detail, all functionality that was previously in initialize_metal_chemistry_rates.cpp has been split between init_misc_species_cool_rates.cpp and collisional_rate_props.cpp.

  • We moved all logic relating to initializing 2body reaction rates (i.e. for primordial_chemistry 4 & metal chemistry) from initialize_metal_chemistry_rates.cpp into collisional_rate_props.cpp.
  • We moved all other logic (that namely pertains to initializing species-specific cooling) out of initialize_metal_chemistry_rates.cpp and into init_misc_species_cool_rates.cpp. (this file probably needs more cleanup, but that's beyond the scope of this PR

Let's talk more about collisional_rate_props.cpp:

As part of this PR:

  • I broke out the logic for each primordial_chemistry==4 and metal_chemistry collisional rate. These functions are analogous to the existing rate functions (k1_rate, k2_rate, ...)1
  • I created a grackle::impl::visit_rate_props function that effectively iterates through all standardized collisional reaction rate functions. This includes all of the existing primordial_chemistry= 1,2, and 3 functions as well as the new primordial_chemistry=4 and metal_chemistry=1 rates.
  • I have slightly refactored the logic contained within the initialize_rates.cpp to make use of grackle::impl::visit_rate_props to actually initialize the standardized collisional reaction rate functions.

Overall, I think this leads to much cleaner code. Now that we have ~882 of the standardized collisional reaction rate functions. I have structured it in the current manner for reasons that should become apparent once #415 is finished

Footnotes

  1. I left comments that explain the work that should be done before we are ready to expose all the new rate functions as part of the public API

  2. At the moment, I'm taking a very broad definition of "standard". In the future, one might imagine treating a handful of rates more specially again. Examples of such rates include: k13 (due to the existence of k13dd), k56 (since we use it to infer a deuterium rate), as well as the 3 body rates.

@mabruzzo mabruzzo changed the base branch from main to newchem-cpp September 6, 2025 20:22
@mabruzzo mabruzzo force-pushed the cleanup-kcol_rate_tables_init branch from 6cef850 to 046e49e Compare September 6, 2025 20:25
@mabruzzo mabruzzo changed the title [newchem-cpp] WIP: Cleanup initialization of kcol_rate_tables [newchem-cpp] Cleanup initialization of kcol_rate_tables Sep 9, 2025
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Sep 9, 2025
@mabruzzo mabruzzo marked this pull request as ready for review September 9, 2025 16:14
@mabruzzo mabruzzo moved this to Awaiting Review in New Chemistry and C++ Transcription Sep 12, 2025
@mabruzzo

Copy link
Copy Markdown
Collaborator Author

@brittonsmith -- this should be ready to go

Comment on lines +364 to +376
// The following comment has been adapted from earlier versions of Grackle
// The rate comes from an experiment (297-3532 K).
// We refrain to extrapolate it to high temperatures.
//
// There are 2 issues here (that were not apparent in older Grackle versions):
// 1. Does this comment just apply to kz22? Or does it apply to kz22 and some
// of the subsequent rates? (which ones?) Or all of the subsequent rates?
// 2. The statement "We refrain to extrapolate it to high temperatures" needs
// to be clarified...
// - I assume that it should read "we refrain from extrapolating it to high
// temperatures"
// - but, we don't obviously do that. It seems like we use the values at ALL
// temperatures.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done a much deeper literature search than I thought I would (more than 10 papers) and have yet to find a reference to this experiment or that temperature range. Dalgarno & Black (1976) equation 2.28 had several references that I chased down, but at that point I can no longer see the rate appear in the same functional form.

This all begins with Omukai (2000), which is the primary source for nearly all the gas-phase reaction rates (as I think this model was explicitly derived from that). Nonetheless, I agree that I see no evidence that we are applying any form of truncation of this rate.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPDATE: a googling of the temperature range itself turned up this:
https://www.aanda.org/articles/aa/full_html/2017/11/aa30388-17/aa30388-17.html
I don't have time to dig deeper into the context here, but it looks like this comes from a UMIST database (which is ironically the first place I went after Omukai 2000.

Again, it doesn't look like we're actually doing anything with this information. I'm going to leave this here as I really REALLY have to get home now. Feel free to update the comment (or not) and I'll merge this in the morning.

@brittonsmith brittonsmith merged commit 26bdc19 into grackle-project:newchem-cpp Nov 13, 2025
5 checks passed
@github-project-automation github-project-automation Bot moved this from Awaiting Review to Done in New Chemistry and C++ Transcription Nov 13, 2025
@mabruzzo mabruzzo deleted the cleanup-kcol_rate_tables_init branch November 13, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor internal reorganization or code simplification with no behavior changes

Projects

Development

Successfully merging this pull request may close these issues.

2 participants