[newchem-cpp] Transcribe calc_tdust_1d_g by ChristopherBignamini · Pull Request #516 · grackle-project/grackle

ChristopherBignamini · 2026-03-12T15:55:11Z

No description provided.

ChristopherBignamini · 2026-03-12T15:56:09Z

pre-commit.ci autofix

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

ChristopherBignamini · 2026-03-12T16:19:46Z

pre-commit.ci autofix

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

…transcribe_calc_tdust_1d_g

mabruzzo · 2026-04-02T12:34:32Z

Wait, does this include transcriptions for both calc_tdust_1d_g and calc_all_tdust_gasgr_1d_g? Or is there a separate PR where you transcribed calc_all_tdust_gasgr_1d_g?

mabruzzo · 2026-04-02T12:36:17Z

Never mind -- I see that calc_all_tdust_gasgr_1d_g was transcribed in #485

mabruzzo · 2026-04-02T14:20:43Z

@ChristopherBignamini, can you merge in the recent changes from newchem-cpp (I suspect that a lot of these conflicts will go away if you perform the merge on your local machine)

ChristopherBignamini · 2026-04-02T15:17:30Z

@mabruzzo I tried to replace strcpy with memcpy, I will revert this proposed change and merge newchem-cpp

…transcribe_calc_tdust_1d_g

mabruzzo

I made a few very minor suggestions. Feel free to push back on whatever you want. Afterwards, this is good to go

mabruzzo · 2026-04-02T16:29:08Z

-  // NOTE: gr_N and gr_Size are historical names
-  // -> they are pretty uninformative and should be changed!
+  // NOTE: gr_N is a historical names
+  // -> it is pretty uninformative and should be changed!
  int gr_N[2] = {0, 0};
-  int gr_Size = 0;


mabruzzo · 2026-04-02T16:31:42Z

+/// @param[in]  gr_dT Temperature spacing of the grain opacity table
+/// @param[in]  gr_Td Temperature values of the grain opacity table
+/// @param[in]  alsp_data_ Grain opacity table data
+/// @param[in]  kgr Array to hold computed grain opacities


Suggested change

/// @param[in] kgr Array to hold computed grain opacities

/// @param[out] kgr Array to hold computed grain opacities

mabruzzo · 2026-04-02T16:36:38Z

+  std::vector<gr_mask_type> nm_itmask(in);
+  std::vector<gr_mask_type> bi_itmask(in);


Relatively unimportant suggestion: (feel free to ignore it)

Suggested change

std::vector<gr_mask_type> nm_itmask(in);

std::vector<gr_mask_type> bi_itmask(in);

// iteration mask specifies where we use newton's method with finite

// differences (aka the secant method)

std::vector<gr_mask_type> nm_itmask(in);

// iteration mask specifies where we use bisection

std::vector<gr_mask_type> bi_itmask(in);

mabruzzo · 2026-04-02T16:37:37Z

+
+  c_done = 0;
+  nm_done = 0;
+  c_total = idx_range.i_end - idx_range.i_start + 1;


Suggested change

c_total = idx_range.i_end - idx_range.i_start + 1;

c_total = idx_range.i_stop - idx_range.i_start;

mabruzzo · 2026-04-02T16:40:37Z

+
+    for (i = idx_range.i_start; i <= idx_range.i_end; i++) {
+      if (nm_itmask[i] != MASK_FALSE) {
+        // Use Newton's method to solve for Tdust


Suggested change

// Use Newton's method to solve for Tdust

// check if the solution has converged (if not prepare the next guess)

mabruzzo · 2026-04-02T17:48:56Z

+  trad = std::fmax(
+      1.,
+      trad);  // TODO: do we really want to write in a passed by copy input par?


Good Point! Maybe we rename the value used throughout the function floored_trad or something? (or maybe we rename the input parameter). I trust your judgement

mabruzzo · 2026-04-02T18:25:34Z

+  std::vector<double> tdplus(in);
+  std::vector<double> tdustnow(in);
+  std::vector<double> tdustold(in);
+  std::vector<double> pert(in);


Relatively unimportant suggestion: (feel free to ignore it)

Suggested change

std::vector<double> tdplus(in);

std::vector<double> tdustnow(in);

std::vector<double> tdustold(in);

std::vector<double> pert(in);

// holds dust temperature guess from the last root-finding iteration

std::vector<double> tdustold(in);

// holds dust temperature guess for the current root-finding iteration

std::vector<double> tdustnow(in);

// holds a value offset from the current dust temperature (tdustnow)

// it is used to get a finite difference estimate for the derivative

std::vector<double> tdplus(in);

// relative finite difference step size

std::vector<double> pert(in);

mabruzzo · 2026-04-02T18:30:41Z

+      if (nm_itmask[i] != MASK_FALSE) {
+        // Use Newton's method to solve for Tdust
+
+        slope[i] = (solplus[i] - sol[i]) / (pert[i] * tdustnow[i]);


Suggested change

slope[i] = (solplus[i] - sol[i]) / (pert[i] * tdustnow[i]);

// todo: convert slope to a local variable (there's no reason for it to be a vector)

slope[i] = (solplus[i] - sol[i]) / (pert[i] * tdustnow[i]);

@mabruzzo should we do it now or maybe you prefer to do the change in a later code optimization/refactoring step?

mabruzzo · 2026-04-02T18:38:30Z

+/// modified: January, 2026 by Christopher Bignamini & Matthew Abruzzo; C++ port
+void calc_tdust_1d_g(double* tdust, double* tgas, double* nh, double* gasgr,
+                     const double* gamma_isrfa, const double* isrf,
+                     const gr_mask_type* itmask, double trad, int in, int gr_N,


Would you mind changing the name of the in parameter to something more descriptive, like buf_len or add a todo note to the docstring to remind us to do that?

For context, each of the 1d arrays holds in elements. The name is left over from when we previously passed around in, jn, kn as arguments (i.e. lengths along i-axis, j-axis, & k-axis)

…transcribe_calc_tdust_1d_g

ChristopherBignamini · 2026-04-08T09:18:58Z

pre-commit.ci autofix

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

…scalar

Porting of calc_tdust_1d_g to C++

65d4290

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

bc93036

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

ChristopherBignamini changed the title ~~[newchem-cpp] Transcribe calc tdust 1d g~~ [newchem-cpp] Transcribe calc_tdust_1d_g Mar 12, 2026

ChristopherBignamini added 2 commits March 12, 2026 17:04

Code cleanup

47e5077

const keyword added

7be8250

pre-commit-ci Bot and others added 2 commits March 12, 2026 16:19

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

e5d0c01

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

Missing separator

13e33f4

ChristopherBignamini mentioned this pull request Mar 21, 2026

[newchem-cpp] Transcribe calc_kappa_gr_g #518

Merged

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

38320ae

…transcribe_calc_tdust_1d_g

ChristopherBignamini mentioned this pull request Mar 24, 2026

[newchem-cpp] Transcribe calc_gr_balance_g #519

Merged

ChristopherBignamini added 2 commits March 24, 2026 17:40

Constant definitions fixes

faa86fb

CircleCI fix

5bcbe00

mabruzzo changed the base branch from newchem-cpp to main April 2, 2026 12:31

mabruzzo changed the base branch from main to newchem-cpp April 2, 2026 12:31

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

5d27dba

…transcribe_calc_tdust_1d_g

mabruzzo approved these changes Apr 2, 2026

View reviewed changes

ChristopherBignamini added 4 commits April 8, 2026 10:32

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

0a2918f

…transcribe_calc_tdust_1d_g

Documentation update

05db02d

Documentation update and variable renaming

d9c76db

Documentation added/updated and variables renaming

8e82187

pre-commit-ci Bot and others added 3 commits April 8, 2026 09:19

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

9d6e978

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

Documentation update and array local slope variable transformed into …

dd5bd5b

…scalar

Local variable declaration update

8ade013

ChristopherBignamini and others added 2 commits April 9, 2026 16:22

Comment removed

93bb650

Merge branch 'newchem-cpp' into newchem-cpp_transcribe_calc_tdust_1d_g

c2acf5c

brittonsmith approved these changes Apr 13, 2026

View reviewed changes

brittonsmith merged commit 5b7052e into grackle-project:newchem-cpp Apr 13, 2026
5 checks passed

	/// @param[in] kgr Array to hold computed grain opacities
	/// @param[out] kgr Array to hold computed grain opacities

		std::vector<gr_mask_type> nm_itmask(in);
		std::vector<gr_mask_type> bi_itmask(in);

	c_total = idx_range.i_end - idx_range.i_start + 1;
	c_total = idx_range.i_stop - idx_range.i_start;

	// Use Newton's method to solve for Tdust
	// check if the solution has converged (if not prepare the next guess)

	slope[i] = (solplus[i] - sol[i]) / (pert[i] * tdustnow[i]);
	// todo: convert slope to a local variable (there's no reason for it to be a vector)
	slope[i] = (solplus[i] - sol[i]) / (pert[i] * tdustnow[i]);

Uh oh!

Conversation

ChristopherBignamini commented Mar 12, 2026

Uh oh!

ChristopherBignamini commented Mar 12, 2026

Uh oh!

ChristopherBignamini commented Mar 12, 2026

Uh oh!

mabruzzo commented Apr 2, 2026

Uh oh!

mabruzzo commented Apr 2, 2026

Uh oh!

mabruzzo commented Apr 2, 2026

Uh oh!

ChristopherBignamini commented Apr 2, 2026

Uh oh!

mabruzzo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChristopherBignamini commented Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants