Skip to content

Refactor such that SpLUT is implemented with XMacros#27

Closed
mabruzzo wants to merge 449 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/Implement-SpLUT-with-XMacros
Closed

Refactor such that SpLUT is implemented with XMacros#27
mabruzzo wants to merge 449 commits into
brittonsmith:gen2024from
mabruzzo:gen2024transcribe/Implement-SpLUT-with-XMacros

Conversation

@mabruzzo

Copy link
Copy Markdown

This PR must be reviewed after #26 is merged


Previously, the SpLUT was implemented as a manually maintained list of all possible primordial, metal and dust species.

This PR consists of 3 commits:

  1. I renamed the members of SpLUT corresponding to dust grains so that they are more consistent with the names tracked in grackle_field_data. For the sake of consistency, I made the same modification to members of OnlyGrainSpLUT
    • SpLUT::SiO2D is now SpLUT::SiO2_dust (be aware, this is distinct from the SpLUT::SiO2 entry)
    • SpLUT::reforg is now SpLUT::ref_org_dust
    • SpLUT::volorg is now SpLUT::vol_org_dust
    • SpLUT::H20ice is now SpLUT::H2O_ice_dust
    • all the other dust-grain name entries now end in _dust (e.g. SpLUT::SiM is now SpLUT::SiM_dust)
  2. I divided grackle_field_data_fdatamembers.def into 2 files.1
    1. field_data_evolved_species.def contains all species fields that are evolved by grackle. In other words, these are the primordial species, dust species, and grain species (these all have corresponding entries within SpLUT
    2. field_data_misc_fdatamembers.def contains all of the other fields
  3. Finally, I refactored SpLUT to actually make use of the XMacros from field_data_evolved_species.def

I have manually confirmed that all tests pass

Footnotes

  1. There are other ways I could have accomplished the end-goal without doing this, but they all would require a bunch more work. Plus, I think having 2 files is reasonable since they each contain ~50 entries.

mabruzzo and others added 29 commits March 29, 2025 13:34
…sary now that the cool1d_cloudy_old_tables_g.F bug is fixed

While I haven't tested it (I don't currently have access to an appropriate system), it should be fine since I was originally the one who originally added the flag (and I don't think anyone uses it)
A handful of c functions had a `[-Wreturn-type]` compiler warning where
a function was supposed to return a value but had at least 1 control
flow branch where we didn't return anything.
I realized that the running the code on MacOS machine with
CMAKE_BUILD_TYPE=Release and CMAKE_BUILD_TYPE=Debug produces slightly
different answers.

The flags used in these cases are:

  - Release: -O3
  - Debug: -g

This is fairly disturbing since it should not make a difference (-O3
does not enable any floating point optimizations). While I haven't
tested it, I'm **extremely** confident that this isn't a CMake issue.

But I'm not really sure what we can do... Let's keep track of this
…cution

Make `pytest` invokable from the root directory
Co-authored-by: Britton Smith <brittonsmith@gmail.com>
Co-authored-by: Britton Smith <brittonsmith@gmail.com>
…tructions

Add instructions for running the corelib tests
…atPrecision

Add a consistency check for the precision of gr_float
…ies.data_path.grackle_data_dir`` in external code.

The whole premise is that PR grackle-project#237 is proposing an alternative approach that for managing data file locations, and I would prefer not to have multiple similar ways to do something like managing data files:
- I happen to think that the alternative is much more robust (i.e. it does not require that a source directory is maintained in perpetuity and that the correct version of the source directory is checked out)
- importantly, it is an approach that will work in C, C++, Fortran, and Python

If we ultimately decide not to use the approach presented in PR grackle-project#237, we can always expose this part of the API later.
CI: shift core-library tests to a separate "job"
mabruzzo and others added 27 commits May 30, 2025 13:42
Co-authored-by: Britton Smith <brittonsmith@gmail.com>
Merge 'ncc-speedup-test_models' into ncc-answers
…_models

[newchem-cpp] Speedup test_models.py by direct importing
[newchem-cpp] Add model tests for newchem-cpp functionality
The intention here is to merge this commit into the main branch while we
configure pre-commit to run code formatting/linting in the newchem-cpp
branch. PR grackle-project#287 (where we start configuring pre-commit to run
formatting/linting) is configured so all changes build off this commit
(in other words, there won't be any merge conflicts when we ultimately
merge newchem-cpp into main).

If we don't merge a .pre-commit-config.yaml file into the main branch,
then pre-commit.ci will report a failure (since it can't find a
.pre-commit-config.yaml file)
I also fixed some of the errors it found
@mabruzzo

Copy link
Copy Markdown
Author

superseded by grackle-project#353

@mabruzzo mabruzzo closed this Jun 11, 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