Skip to content

internal scratch-buffer interface update#26

Closed
mabruzzo wants to merge 264 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/internal_types_updates
Closed

internal scratch-buffer interface update#26
mabruzzo wants to merge 264 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/internal_types_updates

Conversation

@mabruzzo

Copy link
Copy Markdown

This must be reviewed after #25

This consists of 2 commits that tweak the general interface of the internal-type I have been creating (their nominal role is to carry around collections of scratch buffers). Basically I formalize the idea that each of these types should support the visitor design pattern for visiting the data-members of these types (we were already doing to help with allocation and destruction).
A longer description of the design pattern is provided within the modified files.

This is important for the at least the initial transcription of step_rate_newton_raphson. We may be able to get rid of this machinery down the road.

This machinery could be very useful for aggregating memory allocations, the initial port to GPUs (i.e. copying data to and from the GPU), and debugging (e.g. printing out the contents of the buffers tracked by a particular buffer in a nicely formatted manner)


I have manually confirmed that this passes all tests

this parallel clause is used when converting from comoving to proper
this parallel clause is used when converting from proper to comoving
Removed unused local variables from `calc_tdust_3d_g` that followed the
naming scheme used in solve_rate_cool for tracking the mass density for
the mass densities of various metal species used in dust chemistry.

There was logic to convert between proper and comoving units for each of
these variables, but they were not actually used for anything (which is
a good thing since they were never initialized).

These variable declarations were probably blindly copied from somewhere
else and were never removed.
Removed unused local variables from `calc_tdust_3d_g` that followed the
naming scheme used in solve_rate_cool for tracking the mass density for
tracking the mass densities of various metal species used in metal
chemistry.

There was logic to convert between proper and comoving units for each of
these variables, but they were not actually used for anything (which is
a good thing since they were never initialized).

These variable declarations were probably blindly copied from somewhere
else and were never removed.
`make dep` previously didn't work correctly with some of the C++ files
(I think because I changed the suffix of the names of some headers)
This is the last commit in a sequence where we were refactoring the
local variable declarations. The routine should now work properly with
OpenMP, but we can't test that since OpenMP-support was broken in
earlier commits to the gen2024 branch (before we began the
transcription).

More simplifications can certainly be made, but it is worth holding off
on that until after we have transcribed more of Grackle.
…cessary forward-declarations (since they already exist within grackle.h).
mabruzzo and others added 27 commits April 16, 2025 07:47
Originally I tried to fix the problem properly, by having
grackle_macros.h directly include grackle.h. But, this produced
problems related to the fact that some function are redeclared slightly
differently between grackle.h and within the source files (the
difference is OK since it is just related to the appearance of `const`).
A number of pending PRs gradually fix this disagreement throughtout the
codebase. Thus, we are going to hold off on "fixing it properly" until
those other PRs are merged
[newchem-cpp] Reverting minor changes to abundance corrections
[newchem-cpp] 2 minor bugfixes following last week's merge
I a prior merge, I accidently duplicated "initialize_rates.lo" in
``src/clib/Make.config.objects``. This caused obviously introduced some
linking problems with the classic build-system
While it looks like there is a large deviation from the older results,
Britton previously did a lot of work investigating this. It turns out
that the example is in the regime where Grackle keeps overshooting the
equlibrium state. We just aren't at the "same point of overshoot"
…d-standard

Register temporary gold standard
When I changed the type of `end_int` argument of ``interpolate_3Dz_g``
to use a 64-bit integer, I forgot to modify the declaration used within
the InterpolationTest.Interpolate3Dz unit-test
@mabruzzo

Copy link
Copy Markdown
Author

superseded by grackle-project#333

@mabruzzo mabruzzo closed this May 19, 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.

5 participants