Skip to content

[newchem-cpp] transcribe calc_all_tdust_gasgr_1d_g#485

Merged
brittonsmith merged 20 commits into
grackle-project:newchem-cppfrom
ChristopherBignamini:newchem-cpp_transcribe_calc_all_tdust_gasgr_1d_g
Apr 2, 2026
Merged

[newchem-cpp] transcribe calc_all_tdust_gasgr_1d_g#485
brittonsmith merged 20 commits into
grackle-project:newchem-cppfrom
ChristopherBignamini:newchem-cpp_transcribe_calc_all_tdust_gasgr_1d_g

Conversation

@ChristopherBignamini

Copy link
Copy Markdown
Contributor

No description provided.

@ChristopherBignamini ChristopherBignamini changed the title Newchem cpp transcribe calc all tdust gasgr 1d g [newchem-cpp] transcribe calc_all_tdust_gasgr_1d_g Jan 8, 2026
@ChristopherBignamini

Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@ChristopherBignamini

Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@mabruzzo

mabruzzo commented Jan 9, 2026

Copy link
Copy Markdown
Collaborator

This looks great. There's unfortunately a complication.

The Complication:

I forgot to reiterate that you should be doing the dust chemistry transcription stuff working off of PR #447 (it would probably be even better to work off of #464)

For some context:

  1. (most importantly for this PR), PR [newchem-cpp] introduce GrainMetalInjectPathways type #447 did a bunch of work reorganizing a bunch of "loose" constants from the chemistry_data_storage type (commonly known held by the my_rates variable)
  2. it also adds a bunch of comments to some of the remaining fortran functions

How to fix this

Important

Let me know if you would prefer that I just pull a copy of this branch and fix the issues myself. It would probably only take ~15 minutes

In any event, I think the best path forward is to merge #464 into this PR, and then stack the remaining transcription PRs off of this one.

When you merge #464 onto this PR, there is going to be a minor merge-conflict. The easiest way to solve this involves:

  1. insert "#include "inject_model/grain_metal_inject_pathways.hpp" and #include "opaque_storage.hpp" into the top of calc_all_tdust_gasgr_1d_g.cpp.

  2. insert the following block of code into the top of the calc_all_tdust_gasgr_1d function:

  grackle::impl::GrainMetalInjectPathways* inject_pathway_props =
    my_rates->opaque_storage->inject_pathway_props;

  double dlog10Tdust = 0.0;
  double* log10Tdust_vals = nullptr;

  // NOTE: gr_N and gr_Size are historical names
  // -> they are pretty uninformative and should be changed!
  int gr_N[2] = {0, 0};
  int gr_Size = 0;
  if (inject_pathway_props != nullptr) {
    dlog10Tdust =
      inject_pathway_props->log10Tdust_interp_props.parameter_spacing[0];
    log10Tdust_vals =
      inject_pathway_props->log10Tdust_interp_props.parameters[0];

    gr_N[0] = inject_pathway_props->n_opac_poly_coef;
    gr_N[1] = static_cast<int>(
      inject_pathway_props->log10Tdust_interp_props.dimension[0]);
  };
  gr_Size = gr_N[0] * gr_N[1];
  1. do the following search and replaces:
    • replace my_rates->gr_N with gr_N
    • replace my_rates->gr_Size with gr_Size
    • replace my_rates->gr_dT with dlog10Tdust
    • replace my_rates->gr_Td with log10Tdust_vals

@mabruzzo

Copy link
Copy Markdown
Collaborator

While you are making the changes, it would be great if you could move the file into the dust sub-directory (but no pressure if you don't get to it)

@mabruzzo

Copy link
Copy Markdown
Collaborator

Just an FYI: when you do the merge make sure you use the most up-to-date version of PR #464. There were some annoying merge conflicts between that branch and the main branch that I resolved yesterday

@ChristopherBignamini

Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@mabruzzo mabruzzo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I left a few superficial suggested changes to tweak the format of some comments. But, I'm happy to merge now and deal with those later.

Note for when you resolve the merge conflicts:

  • if #521 has been merged into newchem-cpp, everything will be great
  • if #521 hasn't been merged upstream yet, you should explicitly merge it here before you format the code. (That PR properly fixes a clang-format error, whereas the default automatic fix moves happens to move a // NOLINT comment to a bad location)

@brittonsmith, I'm happy to see this merged

Comment on lines +96 to +97
// -> this is ONLY used when `itmask_metal .eq. MASK_TRUE`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// -> this is ONLY used when `itmask_metal .eq. MASK_TRUE`
// -> this is ONLY used when `itmask_metal[i] == MASK_TRUE`

Comment on lines +114 to +115
// ! in UV, absorption coefficient Q ~ 1 (Goldsmith 2001)
// ! so we use the geometrical cross-section of grains [cgs]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ! in UV, absorption coefficient Q ~ 1 (Goldsmith 2001)
// ! so we use the geometrical cross-section of grains [cgs]
// in UV, absorption coefficient Q ~ 1 (Goldsmith 2001)
// so we use the geometrical cross-section of grains [cgs]

mygisrf[i] = my_rates->gamma_isrf *
my_chemistry->local_dust_to_gas_ratio / dust2gas[i] *
metallicity[i];
// ! correct with the depletion or enhancement of condensation rate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// ! correct with the depletion or enhancement of condensation rate.
// correct with the depletion or enhancement of condensation rate.

/// @param[in] grain_kappa Grain species opacity collection
///
/// @par History
/// modified: January, 2026 by Christopher Bignamini & Matthew Abruzzo; C++ port

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// modified: January, 2026 by Christopher Bignamini & Matthew Abruzzo; C++ port
/// modified: January, 2026 by Christopher Bignamini; C++ port

@brittonsmith brittonsmith merged commit 457289a into grackle-project:newchem-cpp Apr 2, 2026
5 checks passed
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.

3 participants