Skip to content

[newchem-cpp] lookup_cool_rates0d 2/2#379

Merged
brittonsmith merged 11 commits into
grackle-project:newchem-cppfrom
mabruzzo:gen2024transcribe/lookup_cool_rates0d_cleanup
Sep 11, 2025
Merged

[newchem-cpp] lookup_cool_rates0d 2/2#379
brittonsmith merged 11 commits into
grackle-project:newchem-cppfrom
mabruzzo:gen2024transcribe/lookup_cool_rates0d_cleanup

Conversation

@mabruzzo

@mabruzzo mabruzzo commented Aug 20, 2025

Copy link
Copy Markdown
Collaborator

This was originally proposed as brittonsmith#33


This must be reviewed after #378 is merged


This PR finishes the transcription of lookup_cool_rates0d that I started back in #375

This PR can be broadly summarized as the following:

  1. I moved 1200+ lines of logic at the end of lookup_cool_rated0d into a function called species_density_derivatives_0d
    • getting the appropriate function signature took quite a bit of work
    • I needed to apply a bunch of transformations to the code as I worked through it. The commits applying these transformations are very atomic (there was a lot of search-and-replace)
    • as noted in PR [newchem-cpp] transcribe lookup_cool_rates0d 1/2 #378 (and in the functions docstring), Gen seems to have copied a lot of logic from step_rate_g and then made a series of systematic changes. After we transcribe step_rate_g, I have some thoughts on how to reconcile the 2 implementation
    • this function was moved to the chemistry_solver_funcs.hpp file. The idea is to eventually store related logic from lookup_cool_rates1d_g, step_rate_g and rate_timestep_g within this file. The goal is to try to store chemistry-logic called here from the grackle solver. To the extent that it's possible, it would be nice to decouple this logic from the solver's underlying scheme (whether it's the classic partial updates with the backward-difference-formula OR newton-raphson OR something else)
  2. I fused lookup_cool_rates0d and time_deriv_0d::derivatives. The fused function is called time_deriv_0d::derivatives. This has been my intention for a while. The name now much more clearly conveys the purpose of the function (while it invokes lookup_cool_rates1d, the purpose of the function is much different).
  3. I started to pre-allocate a bunch of local variables corresponding to 1-element buffers
  4. I changed the data-structures used throughout time_deriv_0d::derivatives. This involved adding an ugly wrapper function that gets called by step_rate_newton_raphson. That is a short-term hack. My next intention is to finish cleaning up step_newton_raphson (I simply wanted to minimize the amount I changed that function in this PR)
  5. I removed a bunch more cleaning (mostly, I just removed a bunch of now unnecessary local variables)

Overall, I think the end result of this PR is a massive improvement over the original fortran routine (and it has much better documentation). While improvements are still possible,1 I think we're done with transcribing this function.

Future Thoughts

It also would make a lot of sense to refactor the function so that it can compute derivatives for multiple sets of conditions at once (i.e. so it's no longer 0d). Alternatively, it might more sense to more directly compute partial derivatives like $\frac{\partial }{\partial \rho_i} \dot{\rho}_i$ or $\frac{\partial }{\partial T} \dot{\rho}_i$. With a little refactoring that I want to do anyways, these calculations would be trivial

Misc

I have manually confirmed that all tests pass

Footnotes

  1. For example, now that I can see the full picture, I would make some different organizational choices. It also would make a lot of sense to refactor the function so that it can compute derivatives for multiple sets of conditions at once (i.e. so it's no longer 0d). Alternatively, it would might more sense to more directly compute $ \frac{\partial}{\partial \rho_i}$ partial derivatives. With a little refactoring that I want to do anyways, this would become straight-forward for us to do

Other than 2 manual tweaks to the function signature (and the tweak to
the docstring), this was accomplished entirely with the command:

```
sed -f cleanup.sed -i \.backup time_deriv_0d.hpp
```

This command replaced ``time_deriv_0d.hpp`` and stored a backup at
``time_deriv_0d.hpp.backup``.

The command made use of the ``cleanup.sed`` script that contained a
bunch of sed commands. I autogenerated this file. The contents of this
file is shown down below:

```
s/dspdot\[1-1\]/rhosp_dot\[SpLUT::e\]/g
s/dspdot\[2-1\]/rhosp_dot\[SpLUT::HI\]/g
s/dspdot\[3-1\]/rhosp_dot\[SpLUT::HII\]/g
s/dspdot\[4-1\]/rhosp_dot\[SpLUT::HeI\]/g
s/dspdot\[5-1\]/rhosp_dot\[SpLUT::HeII\]/g
s/dspdot\[6-1\]/rhosp_dot\[SpLUT::HeIII\]/g
s/dspdot\[7-1\]/rhosp_dot\[SpLUT::HM\]/g
s/dspdot\[8-1\]/rhosp_dot\[SpLUT::H2I\]/g
s/dspdot\[9-1\]/rhosp_dot\[SpLUT::H2II\]/g
s/dspdot\[10-1\]/rhosp_dot\[SpLUT::DI\]/g
s/dspdot\[11-1\]/rhosp_dot\[SpLUT::DII\]/g
s/dspdot\[12-1\]/rhosp_dot\[SpLUT::HDI\]/g
s/dspdot\[13-1\]/rhosp_dot\[SpLUT::DM\]/g
s/dspdot\[14-1\]/rhosp_dot\[SpLUT::HDII\]/g
s/dspdot\[15-1\]/rhosp_dot\[SpLUT::HeHII\]/g
s/dspdot\[16-1\]/rhosp_dot\[SpLUT::CI\]/g
s/dspdot\[17-1\]/rhosp_dot\[SpLUT::CII\]/g
s/dspdot\[18-1\]/rhosp_dot\[SpLUT::CO\]/g
s/dspdot\[19-1\]/rhosp_dot\[SpLUT::CO2\]/g
s/dspdot\[20-1\]/rhosp_dot\[SpLUT::OI\]/g
s/dspdot\[21-1\]/rhosp_dot\[SpLUT::OH\]/g
s/dspdot\[22-1\]/rhosp_dot\[SpLUT::H2O\]/g
s/dspdot\[23-1\]/rhosp_dot\[SpLUT::O2\]/g
s/dspdot\[24-1\]/rhosp_dot\[SpLUT::SiI\]/g
s/dspdot\[25-1\]/rhosp_dot\[SpLUT::SiOI\]/g
s/dspdot\[26-1\]/rhosp_dot\[SpLUT::SiO2I\]/g
s/dspdot\[27-1\]/rhosp_dot\[SpLUT::CH\]/g
s/dspdot\[28-1\]/rhosp_dot\[SpLUT::CH2\]/g
s/dspdot\[29-1\]/rhosp_dot\[SpLUT::COII\]/g
s/dspdot\[30-1\]/rhosp_dot\[SpLUT::OII\]/g
s/dspdot\[31-1\]/rhosp_dot\[SpLUT::OHII\]/g
s/dspdot\[32-1\]/rhosp_dot\[SpLUT::H2OII\]/g
s/dspdot\[33-1\]/rhosp_dot\[SpLUT::H3OII\]/g
s/dspdot\[34-1\]/rhosp_dot\[SpLUT::O2II\]/g
s/dspdot\[35-1\]/rhosp_dot\[SpLUT::Mg\]/g
s/dspdot\[36-1\]/rhosp_dot\[SpLUT::Al\]/g
s/dspdot\[37-1\]/rhosp_dot\[SpLUT::S\]/g
s/dspdot\[38-1\]/rhosp_dot\[SpLUT::Fe\]/g
s/dspdot\[39-1\]/rhosp_dot\[SpLUT::MgSiO3_dust\]/g
s/dspdot\[40-1\]/rhosp_dot\[SpLUT::AC_dust\]/g
s/dspdot\[41-1\]/rhosp_dot\[SpLUT::SiM_dust\]/g
s/dspdot\[42-1\]/rhosp_dot\[SpLUT::FeM_dust\]/g
s/dspdot\[43-1\]/rhosp_dot\[SpLUT::Mg2SiO4_dust\]/g
s/dspdot\[44-1\]/rhosp_dot\[SpLUT::Fe3O4_dust\]/g
s/dspdot\[45-1\]/rhosp_dot\[SpLUT::SiO2_dust\]/g
s/dspdot\[46-1\]/rhosp_dot\[SpLUT::MgO_dust\]/g
s/dspdot\[47-1\]/rhosp_dot\[SpLUT::FeS_dust\]/g
s/dspdot\[48-1\]/rhosp_dot\[SpLUT::Al2O3_dust\]/g
s/dspdot\[49-1\]/rhosp_dot\[SpLUT::ref_org_dust\]/g
s/dspdot\[50-1\]/rhosp_dot\[SpLUT::vol_org_dust\]/g
s/dspdot\[51-1\]/rhosp_dot\[SpLUT::H2O_ice_dust\]/g
```
In more detail, this function does the following for the
``species_density_derivatives_0d`` function

This commit consists of:
- removing the function's forward declaration from ``time_deriv_0d.hpp``
- annotating the function with ``inline`` (honestly, I forgot to do this
  sooner)
- moving the implementation from ``time_deriv_0d.hpp`` to
  ``chemistry_solver_funcs.hpp``. This was a straight-forward
  cut-and-paste. Since we created this file in this commit:
  - I needed to add some boilerplate stuff to the file
  - I decided that all functions in that file are defined as part of
    the ``grackle::impl::chemistry`` namespace
…_rates, k13dd with the rest of the main_scratch_buf
We made the following changes:
- I split the `dsp` argument into the `rhosp` and `eint` arguments
- I split the `dspdot` argument into `rhosp_dot` and `eint_dot_specific`

The fact that `rhosp_dot` is implemented via the `grackle::impl::SpeciesCollection` type will help down the road with factoring out the duplicated logic from `species_density_derivatives_0d` and `step_rate_g`

To do this, I needed to introduce a temporary wrapper function within `step_rate_newton_raphson`. I also needed to perform search-and-replace upon `species_density_derivatives_0d`
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Aug 20, 2025
@mabruzzo mabruzzo changed the title [newchem-cpp] lookup_cool_rates0d 2/2 [newchem-cpp] lookup_cool_rates0d 2/2 Aug 20, 2025
@mabruzzo mabruzzo moved this to Awaiting Review in New Chemistry and C++ Transcription Aug 20, 2025
@brittonsmith brittonsmith changed the base branch from newchem-cpp to main September 11, 2025 09:01
@brittonsmith brittonsmith changed the base branch from main to newchem-cpp September 11, 2025 09:01
@brittonsmith brittonsmith merged commit 38e5f8a into grackle-project:newchem-cpp Sep 11, 2025
5 checks passed
@github-project-automation github-project-automation Bot moved this from Awaiting Review to Done in New Chemistry and C++ Transcription Sep 11, 2025
@mabruzzo mabruzzo deleted the gen2024transcribe/lookup_cool_rates0d_cleanup branch September 11, 2025 14: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