[newchem-cpp] transcribe make consistent #417

ChristopherBignamini · 2025-09-11T16:39:50Z

No description provided.

ChristopherBignamini · 2025-09-11T16:41:19Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

ChristopherBignamini · 2025-09-11T17:04:15Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

mabruzzo · 2025-09-12T15:26:40Z

src/clib/make_consistent.C

+          iSN0 = my_chemistry->metal_abundances + 1;
+          for (i = my_fields->grid_start[0]; i <= my_fields->grid_end[0]; i++) {
+            Ct[i] = my_rates->SN0_XC[iSN0 - 1] * metal(i, j, k);
+            Ot[i] = my_rates->SN0_XO[iSN0 - 1] * metal(i, j, k);
+            Mgt[i] = my_rates->SN0_XMg[iSN0 - 1] * metal(i, j, k);
+            Alt[i] = my_rates->SN0_XAl[iSN0 - 1] * metal(i, j, k);
+            Sit[i] = my_rates->SN0_XSi[iSN0 - 1] * metal(i, j, k);
+            St[i] = my_rates->SN0_XS[iSN0 - 1] * metal(i, j, k);
+            Fet[i] = my_rates->SN0_XFe[iSN0 - 1] * metal(i, j, k);
+
+            Cg[i] = my_rates->SN0_fC[iSN0 - 1] * metal(i, j, k);
+            Og[i] = my_rates->SN0_fO[iSN0 - 1] * metal(i, j, k);
+            Mgg[i] = my_rates->SN0_fMg[iSN0 - 1] * metal(i, j, k);
+            Alg[i] = my_rates->SN0_fAl[iSN0 - 1] * metal(i, j, k);
+            Sig[i] = my_rates->SN0_fSi[iSN0 - 1] * metal(i, j, k);
+            Sg[i] = my_rates->SN0_fS[iSN0 - 1] * metal(i, j, k);
+            Feg[i] = my_rates->SN0_fFe[iSN0 - 1] * metal(i, j, k);
+          }


I think that this suggestion properly addresses all relevant off-by-one indexing

Suggested change

iSN0 = my_chemistry->metal_abundances + 1;

for (i = my_fields->grid_start[0]; i <= my_fields->grid_end[0]; i++) {

Ct[i] = my_rates->SN0_XC[iSN0 - 1] * metal(i, j, k);

Ot[i] = my_rates->SN0_XO[iSN0 - 1] * metal(i, j, k);

Mgt[i] = my_rates->SN0_XMg[iSN0 - 1] * metal(i, j, k);

Alt[i] = my_rates->SN0_XAl[iSN0 - 1] * metal(i, j, k);

Sit[i] = my_rates->SN0_XSi[iSN0 - 1] * metal(i, j, k);

St[i] = my_rates->SN0_XS[iSN0 - 1] * metal(i, j, k);

Fet[i] = my_rates->SN0_XFe[iSN0 - 1] * metal(i, j, k);

Cg[i] = my_rates->SN0_fC[iSN0 - 1] * metal(i, j, k);

Og[i] = my_rates->SN0_fO[iSN0 - 1] * metal(i, j, k);

Mgg[i] = my_rates->SN0_fMg[iSN0 - 1] * metal(i, j, k);

Alg[i] = my_rates->SN0_fAl[iSN0 - 1] * metal(i, j, k);

Sig[i] = my_rates->SN0_fSi[iSN0 - 1] * metal(i, j, k);

Sg[i] = my_rates->SN0_fS[iSN0 - 1] * metal(i, j, k);

Feg[i] = my_rates->SN0_fFe[iSN0 - 1] * metal(i, j, k);

}

iSN0 = my_chemistry->metal_abundances;

for (i = my_fields->grid_start[0]; i <= my_fields->grid_end[0]; i++) {

Ct[i] = my_rates->SN0_XC[iSN0] * metal(i, j, k);

Ot[i] = my_rates->SN0_XO[iSN0] * metal(i, j, k);

Mgt[i] = my_rates->SN0_XMg[iSN0] * metal(i, j, k);

Alt[i] = my_rates->SN0_XAl[iSN0] * metal(i, j, k);

Sit[i] = my_rates->SN0_XSi[iSN0] * metal(i, j, k);

St[i] = my_rates->SN0_XS[iSN0] * metal(i, j, k);

Fet[i] = my_rates->SN0_XFe[iSN0] * metal(i, j, k);

Cg[i] = my_rates->SN0_fC[iSN0] * metal(i, j, k);

Og[i] = my_rates->SN0_fO[iSN0] * metal(i, j, k);

Mgg[i] = my_rates->SN0_fMg[iSN0] * metal(i, j, k);

Alg[i] = my_rates->SN0_fAl[iSN0] * metal(i, j, k);

Sig[i] = my_rates->SN0_fSi[iSN0] * metal(i, j, k);

Sg[i] = my_rates->SN0_fS[iSN0] * metal(i, j, k);

Feg[i] = my_rates->SN0_fFe[iSN0] * metal(i, j, k);

}

mabruzzo · 2025-09-12T15:28:28Z

src/clib/make_consistent.C

+
+namespace grackle::impl {
+
+void make_consistent(const int* imetal, const double* dom,


would you mind refactoring so that imetal and dom are passed by value (and not passed by pointer)?

For reference, imetal is a regular integer and dom is a regular double

mabruzzo · 2025-09-12T15:29:28Z

src/clib/make_consistent.C

+//===----------------------------------------------------------------------===//
+///
+/// @file
+/// Declares signature of make_consistent_g


Suggested change

/// Declares signature of make_consistent_g

/// Implements the `make_consistent_g` function

mabruzzo · 2025-09-12T15:43:49Z

src/clib/make_consistent.C

+          SN_i[1 - 1] = 1;
+          //_// PORT:             SN_metal(:, 1) = metal_loc(:,j,k)
+          SN_i[2 - 1] = 2;
+          //_// PORT:             SN_metal(:, 2) = metal_C13(:,j,k)
+          SN_i[3 - 1] = 3;
+          //_// PORT:             SN_metal(:, 3) = metal_C20(:,j,k)
+          SN_i[4 - 1] = 4;
+          //_// PORT:             SN_metal(:, 4) = metal_C25(:,j,k)
+          SN_i[5 - 1] = 5;
+          //_// PORT:             SN_metal(:, 5) = metal_C30(:,j,k)
+          SN_i[6 - 1] = 6;
+          //_// PORT:             SN_metal(:, 6) = metal_F13(:,j,k)
+          SN_i[7 - 1] = 7;
+          //_// PORT:             SN_metal(:, 7) = metal_F15(:,j,k)
+          SN_i[8 - 1] = 8;
+          //_// PORT:             SN_metal(:, 8) = metal_F50(:,j,k)
+          SN_i[9 - 1] = 9;
+          //_// PORT:             SN_metal(:, 9) = metal_F80(:,j,k)
+          SN_i[10 - 1] = 10;
+          //_// PORT:             SN_metal(:,10) = metal_P170(:,j,k)
+          SN_i[11 - 1] = 11;
+          //_// PORT:             SN_metal(:,11) = metal_P200(:,j,k)
+          SN_i[12 - 1] = 12;
+          //_// PORT:             SN_metal(:,12) = metal_Y19(:,j,k)


As a reminder any line that starts with "//_// PORT: " is a line that the transcription script could not transcribe. Consequently, fixing the highlighted region will probably fix the failing tests

mabruzzo · 2025-09-12T15:45:22Z

src/clib/CMakeLists.txt

  # C++ Source (and Private Header Files)
  cool_multi_time_g-cpp.C cool_multi_time_g-cpp.h
  internal_types.C internal_types.hpp
+  make_consistent.C make_consistent.hpp


You should probably rename make_consistent.C to make_consistent.cpp. That is the new convention that we're adopting. But, I can fix that later if you prefer

… implemented manually

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

ChristopherBignamini · 2025-09-18T11:56:07Z

pre-commit.ci autofix

ChristopherBignamini · 2025-09-18T12:09:11Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

…hem-cpp_transcribe_make_consistent

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

mabruzzo · 2025-09-18T15:00:51Z

I think the test is failing because you forgot to add make_consistent.lo to src/clib/Make.config.objects

mabruzzo · 2025-09-18T15:09:24Z

src/clib/make_consistent.cpp

+        correctH =
+            (gr_float)(my_chemistry->HydrogenFractionByMass * metalfree[i] /
+                       totalH[i]);  // TODO: which one is correct?
+        // correctH = (gr_float)(my_chemistry->HydrogenFractionByMass*(1. -
+        // my_chemistry->DeuteriumToHydrogenRatio)*metalfree[i]/totalH[i]
+        //              );
+        // // ! GC202005
+        // //- !       correctH = real(fh*metalfree(i)/totalH(i), RKIND)


What's going on here? Did you manually add these comments yourself?

This logic doesn't seem to be present in the original version of the function...

I know that there were some changes to the make_consistent_g at one point. Is there a chance you transcribed an old version of the branch rather than the most recent version?

Transcription of make_consistent_g

989bc42

pre-commit-ci bot and others added 3 commits September 11, 2025 16:41

[pre-commit.ci] auto fixes from pre-commit.com hooks

15f8b21

for more information, see https://pre-commit.ci

Unused variables removed

fa18aeb

Merge branch 'newchem-cpp_transcribe_make_consistent' of https://gith…

f938389

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

[pre-commit.ci] auto fixes from pre-commit.com hooks

03a1e17

for more information, see https://pre-commit.ci

mabruzzo reviewed Sep 12, 2025

View reviewed changes

mabruzzo mentioned this pull request Sep 12, 2025

Transcription Tracking Issue #256

Open

32 tasks

ChristopherBignamini added 5 commits September 18, 2025 13:14

Function interface update and missing code, not automatically ported,…

8a599b9

… implemented manually

call to make_consistent updated accorgingly to the new signature

a0ca59e

make_consistent source file renamed

692af6b

file renamed as make_consistent.cpp

425f6e4

Merge branch 'newchem-cpp_transcribe_make_consistent' of https://gith…

17d3bb4

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

Merge branch 'newchem-cpp' into newchem-cpp_transcribe_make_consistent

0ca2321

pre-commit-ci bot and others added 3 commits September 18, 2025 12:10

[pre-commit.ci] auto fixes from pre-commit.com hooks

f675972

for more information, see https://pre-commit.ci

Merge remote-tracking branch 'remotes/upstream/newchem-cpp' into newc…

738c9aa

…hem-cpp_transcribe_make_consistent

Merge branch 'newchem-cpp_transcribe_make_consistent' of https://gith…

c0e6048

…ub.com/ChristopherBignamini/grackle into newchem-cpp_transcribe_make_consistent

mabruzzo reviewed Sep 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[newchem-cpp] transcribe make consistent #417

[newchem-cpp] transcribe make consistent #417

ChristopherBignamini commented Sep 11, 2025

Uh oh!

ChristopherBignamini commented Sep 11, 2025

Uh oh!

ChristopherBignamini commented Sep 11, 2025

Uh oh!

mabruzzo Sep 12, 2025

Uh oh!

mabruzzo Sep 12, 2025

Uh oh!

mabruzzo Sep 12, 2025

Uh oh!

mabruzzo Sep 12, 2025

Uh oh!

mabruzzo Sep 12, 2025

Uh oh!

ChristopherBignamini commented Sep 18, 2025

Uh oh!

ChristopherBignamini commented Sep 18, 2025

Uh oh!

mabruzzo commented Sep 18, 2025

Uh oh!

mabruzzo Sep 18, 2025

Uh oh!

mabruzzo Sep 18, 2025

Uh oh!

Uh oh!


		namespace grackle::impl {

		void make_consistent(const int* imetal, const double* dom,

	/// Declares signature of make_consistent_g
	/// Implements the `make_consistent_g` function

[newchem-cpp] transcribe make consistent #417

Are you sure you want to change the base?

[newchem-cpp] transcribe make consistent #417

Conversation

ChristopherBignamini commented Sep 11, 2025

Uh oh!

ChristopherBignamini commented Sep 11, 2025

Uh oh!

ChristopherBignamini commented Sep 11, 2025

Uh oh!

mabruzzo Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

mabruzzo Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

mabruzzo Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

mabruzzo Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

mabruzzo Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

ChristopherBignamini commented Sep 18, 2025

Uh oh!

ChristopherBignamini commented Sep 18, 2025

Uh oh!

mabruzzo commented Sep 18, 2025

Uh oh!

mabruzzo Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

mabruzzo Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!