Skip to content

[newchem-cpp] lightly refactor calculate_temperature.c#382

Merged
brittonsmith merged 4 commits into
grackle-project:newchem-cppfrom
mabruzzo:gen2024transcribe/calculate_temperature_tweaks
Sep 12, 2025
Merged

[newchem-cpp] lightly refactor calculate_temperature.c#382
brittonsmith merged 4 commits into
grackle-project:newchem-cppfrom
mabruzzo:gen2024transcribe/calculate_temperature_tweaks

Conversation

@mabruzzo

Copy link
Copy Markdown
Collaborator

This was originally proposed as brittonsmith#36


This must be reviewed after #380 is merged.


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 Update linux-gnu make file for apt-get paths. #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

This slightly reorganizes a little logic (for reasons that will become
clear in the next patch). I also replaced a bunch of uses of SUCCESS and
FAIL with GR_SUCCESS and GR_FAIL
Manually inline ``local_calculate_temperature_table`` within
``local_calculate_temperature``. Thanks to recent work, this doesn't
increase the size of the ``local_calculate_temperature`` function
Change occurences of max to fmax so we could eliminate the `include
"grackle_macros.h"` directive
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Aug 20, 2025
@mabruzzo mabruzzo moved this to Awaiting Review in New Chemistry and C++ Transcription Aug 20, 2025
@mabruzzo mabruzzo changed the base branch from newchem-cpp to main September 11, 2025 18:11
@mabruzzo mabruzzo changed the base branch from main to newchem-cpp September 11, 2025 18:11
@brittonsmith brittonsmith changed the base branch from newchem-cpp to main September 12, 2025 10:33
@brittonsmith brittonsmith changed the base branch from main to newchem-cpp September 12, 2025 10:34
@brittonsmith brittonsmith merged commit 3118978 into grackle-project:newchem-cpp Sep 12, 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 12, 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

refactor internal reorganization or code simplification with no behavior changes

Projects

Development

Successfully merging this pull request may close these issues.

2 participants