Skip to content

transcribe lookup_cool_rates0d 1/2#31

Closed
mabruzzo wants to merge 678 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/lookup_cool_rates0d
Closed

transcribe lookup_cool_rates0d 1/2#31
mabruzzo wants to merge 678 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/lookup_cool_rates0d

Conversation

@mabruzzo

@mabruzzo mabruzzo commented Feb 14, 2025

Copy link
Copy Markdown

This PR must be reviewed after #30


This was the step 1 of 2 for transcribing lookup_cool_rates0d.

  • the first 3 commits tweaked the fortran code. Prior to this PR some constant values were passed directly as arguments (in these cases, we used values of 0 or 1). Essentially, we introduced a few local variables to temporarily store these values and then passed the variables as arguments (this overcomes a limitation of the transcription tool)
  • then we performed the initial transcription
  • After doing the initial transcription, and reducing some most work was effort was devoted to refactoring all of the local variable so we could replace the calls to FORTRAN_NAME(cool1d_multi_g), FORTRAN_NAME(lookup_cool_rates1d_g), FORTRAN_NAME(rate_timestep_g) with the C++ wrappers (that use the reduced arg lists)
    • for example, historically the routine stored made a copy of every species mass density (and the internal energy) and stored the copies within local variables. Since all of our C++ wrappers expect this information to be specified within an instance of grackle_field_data, this needed to change:
      • several commits were storing the copies within the pointers specified by an instance of grackle_field_data (distinct from the instance passed to Grackle's API).
      • The local variables previously dedicated to holding the local copies were converted to references to the local copies. This let us avoid breaking the 1200+ lines of logic at the end of the function. Said logic was clearly copied from step_rate_g and then heavily modified. That logic is more directly addressed in the next PR
    • I also replaced >100 local variables with instances of the ColRecRxnRateCollection, PhotoRxnRateCollection, and GrainSpeciesCollection types since the C++ use these types to pass around large bundles of quantities (there was a bunch of search-and-replace done to avoid breaking logic at the end of the function)
    • I replaced a couple of local variables with an instance of IndexRange
  • I also removed a bunch of unused local variables

The that the cool1d_multi_g, lookup_cool_rates1d_g, and rate_timestep_g routines are only called from C++ source files AND the fact they are only called through the C++ wrappers (i.e. grackle::impl::fortran_wrapper::cool1d_multi_g, grackle::impl::fortran_wrapper::lookup_cool_rates1d_g, and grackle::impl::rate_timestep_g) is an important milestone. It means that they can be replaced with transcribed function.


The resulting code in this PR is still very messy. But, I choose to make this PR at this point because it was a point when @ChristopherBignamini could working from this branch in order to transcribe cool1d_multi_g in parallel with my efforts. In fact, it was originally my intention to break this particular PR into smaller (slightly more logical) pieces, but I prioritized "getting done" in order to stop being a bottleneck for @ChristopherBignamini

Another PR has been posted to further cleanup lookup_cool_rates0d.


I have confirmed that this PR passes all tests

@mabruzzo mabruzzo changed the base branch from master to gen2024 February 14, 2025 15:31
@mabruzzo mabruzzo changed the title initial transcription of lookup_cool_rates0d transcribe lookup_cool_rates0d 1/2 Feb 16, 2025
mabruzzo and others added 26 commits May 19, 2025 14:59
Originally one job ran the tests after building pygrackle as a
standalone package (i.e. the core Grackle is compiled as part of
pygrackle). The other job ran the tests with versions of pygrackle
where we explicitly built the core library (with both build-systems) as
a separate step from building pygrackle.
In these cases, we only run a subset of the test-suite (the much faster
component). This seems reasonable since we are already running the
answer-tests with the version of pygrackle configured that automatically
compiled the core-library as part of the build process. This should save
a lot of time
…sting-without-editable-install

Support running subset of pygrackle test without a an editable install
…tweak

cython does not need to be a runtime-dependency
…nstruction-tweaks

Pygrackle test instruction tweaks
mabruzzo and others added 27 commits August 12, 2025 19:00
…e/consistency-tweaks

[newchem-cpp]  Consistency tweaks: `GrainSpeciesCollection` and `cool_multi_time_g`
…o gen2024transcribe/step_rate_newton_raphson-cleanup-1of2
…e/internal_types_updates

[newchem-cpp] internal scratch-buffer interface update
…e/Implement-SpLUT-with-XMacros

[newchem-cpp] Refactor such that SpLUT is implemented with XMacros
…e/SpeciesLUTFieldAdaptor

[newchem-cpp] Introduce `SpeciesLutFieldAdaptor`
The *Great Renaming:* `gracklepy` edition
…e/initial-step_rate_newton_raphson

[newchem-cpp] initial transcription of `step_rate_newton_raphson`
…' into gen2024transcribe/lookup_cool_rates0d
@mabruzzo

Copy link
Copy Markdown
Author

superseded by grackle-project#378

@mabruzzo mabruzzo closed this Aug 20, 2025
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

Successfully merging this pull request may close these issues.

3 participants