Skip to content

[newchem-cpp] transcribe calc_temp_cloudy_g#380

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

[newchem-cpp] transcribe calc_temp_cloudy_g#380
brittonsmith merged 10 commits into
grackle-project:newchem-cppfrom
mabruzzo:gen2024transcribe/calc_temp_cloudy_g

Conversation

@mabruzzo

Copy link
Copy Markdown
Collaborator

This PR was originally proposed as brittonsmith#34


This should not be reviewed until after #379 is merged


This PR transcribes calc_temp_cloudy_g from Fortran to C++. At this point the procedure is becoming pretty standardized. It might be easier to review commit-by-commit

The main goal is to construct an instance of `InternalGrUnits` that will
be passed into `calc_temp_cloudy_g`, once it is transcribed.
`local_calculate_cooling_time` and `local_solve_chemistry` similarly
pass instances of `InternalGrUnits` into functions that have already
been transcribed from Fortran.

When I added `#include "internal_units.h"`, I decided to also replace 2
existing include directives with `#include "grackle.h"`
- this is a convention I have been moving us towards (it lets us avoid
  multiple copies of some function declarations and we will be moving
  the replaced headers in future releases)
- I removed all the declarations now made redundant by "grackle.h"
I essentially started to use ``grackle_index_helper`` & ``idx_range``:
- I localized declarations of variables that are provided by these types
- I made the loop bounds over ``i`` start to use idx_range.i_start and
  idx_range.i_stop
I altered the spatial indexing so that it is 0-indexed
…2024transcribe/calc_temp_cloudy_g

I needed to introduce the bugfix from PR grackle-project#367.
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Aug 20, 2025
@mabruzzo mabruzzo changed the base branch from main to newchem-cpp August 20, 2025 15:25
@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:39
@brittonsmith brittonsmith changed the base branch from main to newchem-cpp September 11, 2025 09:39

@brittonsmith brittonsmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing this commit by commit was helpful!

@brittonsmith brittonsmith merged commit f598650 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/calc_temp_cloudy_g branch September 12, 2025 14: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.

2 participants