Skip to content

lightly refactor calculate_temperature.c tweaks#36

Closed
mabruzzo wants to merge 709 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/calculate_temperature_tweaks
Closed

lightly refactor calculate_temperature.c tweaks#36
mabruzzo wants to merge 709 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/calculate_temperature_tweaks

Conversation

@mabruzzo

Copy link
Copy Markdown

This must be reviewed after #34.


This PR does some (largely superficial) refactoring of the calculate_temperature.c file.

There were 3 commits:

  1. I replaced a bunch of occurrences of SUCCESS and FAIL with GR_SUCCESS and GR_FAIL. I also slightly rearranged a little logic within local_calculate_temperature (in preparation for the next commit)
  2. I "inlined" the definition of the local_calculate_temperature_table helper function directly inside of local_calculate_temperature (this allowed me to delete local_calculate_temperature_table). For additional context:
    • When I transcribed calc_temp_cloudy_g in PR transcribe calc_temp_cloudy_g #34, the definition of local_calculate_temperature_table got a lot shorter. With those changes, local_calculate_temperature_table performed an error-check that primordial_chemistry==0 and simply called `calc_temp_cloudy_g
    • Since local_calculate_temperature only called local_calculate_temperature_table when primordial_chemistry==0, inlining the definition of local_calculate_temperature_table within local_calculate_temperature let us eliminate all associated error-handling logic
    • Consequently, this commit did not increase the number of lines of local_calculate_temperature (or make the logic any harder to follow). At the same time, it allowed us to remove all of the lines of code where we had previously declared/defined local_calculate_temperature_table
  3. Finally, I replaced some occurrences of the max macro with C's fmax function. At this point, I was able to remove the include "grackle_macros.h" line

I manually confirmed that all tests pass for this PR

mabruzzo and others added 30 commits May 19, 2025 22:43
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
…Warnings

Enable Compiler Warnings in a CI job
…lding_length_fix

H2 self shielding length scale factor fix
Superficial Website Documentation Tweaks
[newchem-cpp] Minor model test refactor and adding model comparison option
mabruzzo and others added 26 commits August 13, 2025 08:36
…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
…2024transcribe/calc_temp_cloudy_g

I needed to introduce the bugfix from PR grackle-project#367.
@mabruzzo

Copy link
Copy Markdown
Author

superseded by grackle-project#382

@mabruzzo mabruzzo closed this Aug 20, 2025
@mabruzzo mabruzzo deleted the gen2024transcribe/calculate_temperature_tweaks branch September 12, 2025 14:07
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