Skip to content

Conversation

mabruzzo
Copy link
Collaborator

@mabruzzo mabruzzo commented Sep 10, 2025

To be reviewed after #415 is merged


The primary objective of this PR is to factor out heap allocations from the core loop. To accomplish this, this PR

  • pre-initializes the grid of $\ln T_{\rm gas}$ and $\ln T_{\rm dust}$ values that are used to interpolate h2dustS and h2dustC.
  • factors out some shielding logic, into ShieldFactorCalculator, so that we can avoid allocating the f_shieldH and f_shieldHe arrays)
  • we now pre-allocate the internal_dust_prop_buf instance

While I was doing this, I also factored out the H2-shielding logic into a helper function to try to improve readability (and make the code easier to reason about). I was also able to remove a bunch of logic where we were constructing Vlews from the body of lookup_cool_rate1d.

These deleted declarations already occur within the grackle.h header.
Some include directives were also deleted that were redundant with
including then grackle.h header
This is important once we start compiling with C++
For context, the historic convention in Grackle project was to use .C as
the suffix for C++ source files. Thus, when we began transcribing files,
we used the .C suffix (even though it is a less common choice than .cpp)

However, it turns out that converting a file named `<prefix>.c` to
`<prefix>.C` causes lots of issues performing Git operations when your
machine has a case-insensitive file-system (common on macOS). This comes
up in operations as simple as changing between 2 branches

Consequently, we will be transitioning to the .cpp suffix
For context, the historic convention in Grackle project was to use .C as
the suffix for C++ source files. Thus, when we began transcribing files,
we used the .C suffix (even though it is a less common choice than .cpp)

However, it turns out that converting a file named `<prefix>.c` to
`<prefix>.C` causes lots of issues performing Git operations when your
machine has a case-insensitive file-system (common on macOS). This comes
up in operations as simple as changing between 2 branches

Consequently, we will be transitioning to the .cpp suffix
I also adjusted the boilerplate at the top of this file and at the top
of solve_rate_cool_g-cpp.h
A few tweaks were necessary:
- I needed to add the `extern "C"` annotation to a number of functions
  that are publicly exposed as the C API
- I removed the leading underscore from some internal helper functions
- I needed to tweak make the group name argument passed to
  initialize_cloudy_data expect a `const char*`, rather than just a
  `char*`. This is essential since we are passing a string literal (this
  was a very minimal change)

I also added to the `static` specifier to some internal functions (this
ensures that these helper functions will not be accessible to external
code)
I also renamed initialize_rates.h to initialize_rates.hpp to denote that
the header is **ONLY** compatible with C++ source files
This information has been formatted into a markdown table (that will be
rendered into a table if/when we use doxygen)
@mabruzzo mabruzzo changed the base branch from newchem-cpp to main September 12, 2025 15:53
@mabruzzo mabruzzo changed the base branch from main to newchem-cpp September 12, 2025 15:54
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Sep 12, 2025
@mabruzzo mabruzzo changed the title [newchem-cpp] WIP: lookup_cool_rate1d cleanup part 2 [newchem-cpp] lookup_cool_rate1d cleanup part 2 Sep 12, 2025
@mabruzzo mabruzzo marked this pull request as ready for review September 12, 2025 16:08
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.

1 participant