Finalize Gen PR #177
Draft
Finalize Gen PR #177
+19,255
−928
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mmit) to initialize_rates.c
…ate_newton_raphson_v2
Tests seem to be passed!
…(these are used internally by step_rate_newton_raphson)
… step_rate_newton_raphson.F
Previously, it was declared as a scalar. I was tipped off to this issue by the fact that `cool1d_multi_g` expected a 1D array. I further confirmed that `solve_rate_cool_g` was passed the exact same array (by checking arg lists within `calculate_cooling_time.c` and `solve_chemistry.c`).
Convert `logical` to `MASK_TYPE` (aka `integer*4`)
factor `step_rate_newton_raphson` out of `solve_rate_cool_g.F`
Gen2024 rank errors
We needed to pass `iH2shieldcustom` and `f_shield_custom` as arguments to `lookup_cool_rates1d_g`. These variables already had type declarations within `lookup_cool_rates1d_g`, but they because they were passed as part of the argument list, they were previously locally allocated within the function and used without initialization. This is all fixed now
Cells at low temperatures previously exceeded the iteration limit. This change should ensure that the tests produce consistent results.
Add logic to initialize itmask_tmp when primordial_chemistry==0. The need for doing this was identified from running cooling_cell.py
This is done to ease transcription
This was done to remove a bunch of `int(<arg>, 8)` casts. These removed casts all occured while passing arguments to `interpolate_Nd_g` routines. Essentially, the Fortran compiler understands that we want to cast an argument and then pass the result into the subroutine by reference. This is tricky to transcribe to C/C++.
Prior to this commit, the `chemistry_data_storage` struct declared a 9
attributes for each species ``s``, in the list: ``H2``, ``HD``, ``CI``,
``CII``, ``OI``, ``CO``, ``OH``, ``H2O``. In particular the declaration
looked like:
```c++
{
long long *Ls_N, Ls_Size;
double *Ls_D, *Ls_T, *Ls_H, Ls_dD, Ls_dT, Ls_dH, *Ls_L;
}
```
This commit replaced the set of 9 attributes for each species with a
struct called ``gr_interp_grid``. This provides a few benefits:
1. we have removed 64 members from ``chemistry_data_storage``
(this alone is probably enough reason for the change simplicity is
better in this complex data structure)
2. we are able to standardize the logic for zero-initializing the data
in the replaced attributes and for freeing any associated memory
3. this will reduce the code we will eventually need to expose this
information through the pygrackle interface
4. after transcribing Fortran to C, this will help with encapsulating
some common operations.
> [!NOTE]
> Technically, most members of ``gr_interp_grid`` are sorted into a
> separate struct called ``gr_interp_grid_props``. The provided comments
> explain that this lays the groundwork for reducing duplication with
> the ``cloudy_data`` struct at some point in the future.
Even if we decide to reorganize this data in the future, it will be easy
to transform it from the new version of the code than it would be before
this commit.
Prior to this commit, there was a collection of 9 members in the `chemistry_data_storage` struct. This commit replaced those 9 members with a single member of type `gr_interp_grid`.
Basically, I factored out all of the common code for each
`initialize_cooling_rate_{s}` function (this function exists for
each member of `chemistry_data_storage` that stores a `gr_interp_grid`
struct).
|
I just wanted to share some quick thoughts that I had while working on this branch. I think we need to make sure we address the following 2 points (or have a concrete plan to address them) before we merge this PR:
Footnotes
|
fwd `iH2shieldcustom` & `f_shield_custom` args
Gen2024 uninitialized err
Removing cases where intermediate values are passed to subroutines
Gen2024 newstruct
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a reissue of PR #143 (originally PR #78). I am going to try to push this forward to getting merged.
To-dos:
I will shortly be soliciting volunteers to help with this.