Skip to content

[newchem-cpp] Hoist mask-modifications and edot initialization out of cool1d_multi_g#546

Open
mabruzzo wants to merge 10 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/hoist-mask-and-edot-init
Open

[newchem-cpp] Hoist mask-modifications and edot initialization out of cool1d_multi_g#546
mabruzzo wants to merge 10 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/hoist-mask-and-edot-init

Conversation

@mabruzzo

@mabruzzo mabruzzo commented May 3, 2026

Copy link
Copy Markdown
Collaborator

To be reviewed after #545


This PR is pretty straight-forward.

The main changes:

  • we now modify itmask (based on the temperature floor) before we enter cool1d_multi_g
  • we now fill itmask_metal before we enter cool1d_multi_g
  • we now initialize edot (i.e. fill with zeros) before we enter cool1d_multi_g

The last change is probably the most controversial change. This change was motivated by the fact that I want to be able to compute all cooling and reaction rates pertaining to our dust model all at once (so we can better compartmentalize that code and significantly improve performance/reduce allocations for Gen's model). At present, the easiest way to handle this will be to call that logic before we call cool1d_multi_g (i.e. when we call cool1d_multi_g, the edot buffer will already include cooling contributions from the dust model).1

This PR and the prior PR (as well as the one before that) should all use the same gold-standard.

Footnotes

  1. If we want to call cool1d_multi_g before the dust calculations, then we're going to need to move the optical-depth adjustments to a separate function called after the dust calculations. If there's a preference for this plan, we can totally do it -- I don't really which care which approach we take.

@brittonsmith brittonsmith changed the base branch from newchem-cpp to main May 21, 2026 09:55
@brittonsmith brittonsmith changed the base branch from main to newchem-cpp May 21, 2026 09:55
@brittonsmith

brittonsmith commented May 21, 2026

Copy link
Copy Markdown
Contributor

I like the extraction of the masking to external functionality, but it might be worth stuffing it into an initialize_masks function so we're not duplicating so much.

I have noted two additional places where metal masking is happening in an identical without the use of itmask_metal. I think we should thread this through there. These are:

  • calc_tdust_3d.cpp
  • make_consistent.cpp
    On a similar note, I also note that calc_tdust_3d.cpp is still doing its own metallicity calculation.

I've now written and rewritten thoughts about extracting the dust component to the edot calculation three times. I was initially not in favor as I was concerned about code duplication to get cooling time, but I'm trying to keep in mind your point about making it easier to modify dust treatments, so I think it makes sense. In any case, I'm willing to explore it and see how the initial refactor plays out.

Update: in light of moving on to PR #547, you can definitely consider the last point above as requiring no action, but let me know your thoughts on the first two points.

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.

2 participants