Skip to content

[newchem-cpp] WIP lookup_cool_rate1d cleanup part 3#424

Closed
mabruzzo wants to merge 160 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/lookup_rate_1d_cleanup-grain
Closed

[newchem-cpp] WIP lookup_cool_rate1d cleanup part 3#424
mabruzzo wants to merge 160 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/lookup_rate_1d_cleanup-grain

Conversation

@mabruzzo

Copy link
Copy Markdown
Collaborator

Shouldn't be reviewed until after #416 is merged


The premise here is to cleanup the dust-grain logic in the function

These deleted declarations already occur within the grackle.h header.
Some include directives were also deleted that were redundant with
including then grackle.h header
This is important once we start compiling with C++
For context, the historic convention in Grackle project was to use .C as
the suffix for C++ source files. Thus, when we began transcribing files,
we used the .C suffix (even though it is a less common choice than .cpp)

However, it turns out that converting a file named `<prefix>.c` to
`<prefix>.C` causes lots of issues performing Git operations when your
machine has a case-insensitive file-system (common on macOS). This comes
up in operations as simple as changing between 2 branches

Consequently, we will be transitioning to the .cpp suffix
For context, the historic convention in Grackle project was to use .C as
the suffix for C++ source files. Thus, when we began transcribing files,
we used the .C suffix (even though it is a less common choice than .cpp)

However, it turns out that converting a file named `<prefix>.c` to
`<prefix>.C` causes lots of issues performing Git operations when your
machine has a case-insensitive file-system (common on macOS). This comes
up in operations as simple as changing between 2 branches

Consequently, we will be transitioning to the .cpp suffix
I also adjusted the boilerplate at the top of this file and at the top
of solve_rate_cool_g-cpp.h
A few tweaks were necessary:
- I needed to add the `extern "C"` annotation to a number of functions
  that are publicly exposed as the C API
- I removed the leading underscore from some internal helper functions
- I needed to tweak make the group name argument passed to
  initialize_cloudy_data expect a `const char*`, rather than just a
  `char*`. This is essential since we are passing a string literal (this
  was a very minimal change)

I also added to the `static` specifier to some internal functions (this
ensures that these helper functions will not be accessible to external
code)
I also renamed initialize_rates.h to initialize_rates.hpp to denote that
the header is **ONLY** compatible with C++ source files
This information has been formatted into a markdown table (that will be
rendered into a table if/when we use doxygen)
This patch stops the allocating `internal_dust_prop_buf` within
`lookup_cool_rates1d`, and passes in the pre-allocated buffers instead.
As part of the commit, I also renamed `internal_dust_prop_buf` so that
it is now called `internal_dust_prop_scratch_buf`
@mabruzzo mabruzzo changed the base branch from main to newchem-cpp September 12, 2025 20:07
@mabruzzo

mabruzzo commented Sep 13, 2025

Copy link
Copy Markdown
Collaborator Author

I'll circle back on this

@mabruzzo mabruzzo closed this Sep 13, 2025
@mabruzzo mabruzzo deleted the ncc/lookup_rate_1d_cleanup-grain branch October 2, 2025 14:18
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.

1 participant