add derived fields to cluster pgen #63

BenWibking · 2023-05-18T18:12:25Z

This adds various derived fields to the cluster pgen:

cell radius
entropy
sonic Mach number
plasma beta
temperature
cooling time

forrestglines · 2023-05-23T00:50:42Z

@par-hermes format

forrestglines · 2023-05-23T01:13:27Z

src/pgen/cluster.cpp

+
+  // get prim vars
+  auto &data = pmb->meshblock_data.Get();
+  auto const &prim = data->Get("prim").data;


Are we sure prim is up-to-date here? Would it be correct at the start of the simulation? Should we add a ConsToPrim in the kernel?

@pgrete is there a flowchart showing the tasks/kernels called during the hydro update? We keep coming back to the question of when prim is consistent with cons in multiple PRs, and it would be good to have a reference/doc to resolve this in general.

Are we sure prim is up-to-date here? Would it be correct at the start of the simulation? Should we add a ConsToPrim in the kernel?

Fairly sure. Because this function is called for the output (i.e., outside of a cycle) and the design/assumption is that data is consistent at the beginning/end of a cycle.

@pgrete is there a flowchart showing the tasks/kernels called during the hydro update? We keep coming back to the question of when prim is consistent with cons in multiple PRs, and it would be good to have a reference/doc to resolve this in general.

Unfortunately, there is not, but at the Athena meeting multiple people requested this feature.
It should be fairly straightforward to automatically create a flowchart with a small PR in Parthenon that dumps the task list in a format that is understood by some graph library.
Any volunteers? ;)

"""
Fairly sure. Because this function is called for the output (i.e., outside of a cycle) and the design/assumption is that data is consistent at the beginning/end of a cycle.
"""
I'm assuming that includes ghost zones too so this isn't the cause for Ascent meshblock boundary artifacts.

"""
It should be fairly straightforward to automatically create a flowchart with a small PR in Parthenon that dumps the task list in a format that is understood by some graph library.
Any volunteers? ;)
"""
Sounds interesting to me at least! I might take a stab at it

""" Fairly sure. Because this function is called for the output (i.e., outside of a cycle) and the design/assumption is that data is consistent at the beginning/end of a cycle. """

I'm assuming that includes ghost zones too so this isn't the cause for Ascent meshblock boundary artifacts.

Correct, ghost cells are up-to-date when outputs are called.

""" It should be fairly straightforward to automatically create a flowchart with a small PR in Parthenon that dumps the task list in a format that is understood by some graph library. Any volunteers? ;) """

Sounds interesting to me at least! I might take a stab at it

For reference, my first attempt/thoughts parthenon-hpc-lab/parthenon#880

src/pgen/cluster.cpp

src/hydro/srcterms/tabular_cooling.cpp

src/pgen/cluster.cpp

Co-authored-by: forrestglines <[email protected]>

src/hydro/srcterms/tabular_cooling.cpp

src/hydro/srcterms/tabular_cooling.hpp

src/pgen/cluster.cpp

pgrete · 2023-05-24T07:52:51Z

src/pgen/cluster.cpp

+
+  // get prim vars
+  auto &data = pmb->meshblock_data.Get();
+  auto const &prim = data->Get("prim").data;


Are we sure prim is up-to-date here? Would it be correct at the start of the simulation? Should we add a ConsToPrim in the kernel?

Fairly sure. Because this function is called for the output (i.e., outside of a cycle) and the design/assumption is that data is consistent at the beginning/end of a cycle.

@pgrete is there a flowchart showing the tasks/kernels called during the hydro update? We keep coming back to the question of when prim is consistent with cons in multiple PRs, and it would be good to have a reference/doc to resolve this in general.

Unfortunately, there is not, but at the Athena meeting multiple people requested this feature.
It should be fairly straightforward to automatically create a flowchart with a small PR in Parthenon that dumps the task list in a format that is understood by some graph library.
Any volunteers? ;)

src/pgen/cluster.cpp

pgrete · 2023-05-24T08:30:51Z

src/pgen/cluster.cpp

+          const Real edot = cooling_table_obj.DeDt(eint, rho);
+          const Real t_cool = eint / edot;


We probably need a check for edot = 0 here.

Is NaN a bad answer here? What should it return in this case?

We really really should since below the cooling table it will be effectively zero. For consistency we could set it to positive infinity.

Ideally we'd treat cold gas separately or ignore it for calculation of 1D profiles. Deovrat might have a more informed opinion.

src/pgen/cluster.cpp

pgrete · 2023-05-24T08:32:35Z

src/pgen/cluster.cpp

+          const Real Pmag = 0.5 * (SQR(B1) + SQR(B2) + SQR(B3));
+
+          // compute plasma beta
+          plasma_beta(0, k, j, i) = Pgas / Pmag;


Should check that if Pmag == 0 we set it to sth tiny.

As before, is NaN a bad answer? What would be more sensible?

I think NAN would be good but hack in NAN exclusion into the Ascent 1d profiles and projects if that's possible

Not sure if that's possible. We'd need to ask the Ascent devs.

Co-authored-by: Philipp Grete <[email protected]>

…thenapk into cluster-derived-vars

forrestglines · 2023-05-24T16:44:12Z

@par-hermes format

src/pgen/cluster.cpp

Piggyback removal of labels for acceleration fields in turb generator as the (new) default ones are effectively identical.

forrestglines · 2023-05-30T05:00:16Z

@par-hermes format

forrestglines · 2023-06-01T22:01:03Z

@par-hermes format

forrestglines · 2023-06-01T22:01:54Z

I think I've fixed the cooling time bug. It looks good from vim-inspection of 1D profile data given by Ascent

This is once again ready for review, given that the CI passes

pgrete

Thanks for the fix. Looks good to me now.

pgrete · 2023-06-02T14:21:21Z

CI passed (though still not reported yet). I'm merging now.

BenWibking added 2 commits May 18, 2023 14:09

add derived fields

ee07132

add Mach number

cbbd32c

BenWibking requested review from forrestglines and pgrete May 18, 2023 18:25

BenWibking and others added 5 commits May 18, 2023 15:02

use log10 radius instead

d64eab1

Added temperature as derived field

ff00032

fix previous commit

6b0012a

Tabular Cooling DeDt refactor

798a737

Added cooling_time derived field

58189c6

cpp-py-formatter

c7b3756

forrestglines approved these changes May 23, 2023

View reviewed changes

Update src/pgen/cluster.cpp

988d884

Co-authored-by: forrestglines <[email protected]>

pgrete requested changes May 24, 2023

View reviewed changes

BenWibking and others added 9 commits May 25, 2023 00:01

Update src/pgen/cluster.cpp

50b3b1b

Co-authored-by: Philipp Grete <[email protected]>

Update src/pgen/cluster.cpp

16fcb8d

Co-authored-by: Philipp Grete <[email protected]>

Update src/pgen/cluster.cpp

f8db69b

Co-authored-by: Philipp Grete <[email protected]>

Update src/pgen/cluster.cpp

42c9e77

Co-authored-by: Philipp Grete <[email protected]>

Update src/pgen/cluster.cpp

969da8e

Co-authored-by: Philipp Grete <[email protected]>

fix indices

f7d391b

Refactor cooling_table_obj, add mach_alfven

a20e636

Removed extra 0, indexers

847b8eb

Merge branch 'cluster-derived-vars' of github.com:parthenon-hpc-lab/a…

981d274

…thenapk into cluster-derived-vars

par-hermes and others added 2 commits May 24, 2023 16:44

cpp-py-formatter

e179053

Explicility set edot==0 -> t_cool=NAN, and M_A

f74f094

forrestglines reviewed May 25, 2023

View reviewed changes

src/pgen/cluster.cpp Outdated Show resolved Hide resolved

pgrete and others added 3 commits May 25, 2023 15:30

Fix calc of alfven mach in cluster output

d4c7f6b

Piggyback removal of labels for acceleration fields in turb generator as the (new) default ones are effectively identical.

WIP: Rolled back tabular cooling

79e50e6

Compute cooling time with old DeDt

031c3df

par-hermes and others added 2 commits May 30, 2023 05:00

cpp-py-formatter

781c726

Reapplied cooling table refactor, fixed bug

4ba221e

cpp-py-formatter

46aa160

pgrete approved these changes Jun 2, 2023

View reviewed changes

pgrete merged commit 8f04e18 into main Jun 2, 2023

BenWibking deleted the cluster-derived-vars branch August 1, 2023 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add derived fields to cluster pgen #63

add derived fields to cluster pgen #63

BenWibking commented May 18, 2023 •

edited

Loading

forrestglines commented May 23, 2023

forrestglines May 23, 2023

BenWibking May 23, 2023 •

edited

Loading

pgrete May 24, 2023

forrestglines May 24, 2023

pgrete May 25, 2023

pgrete May 24, 2023

pgrete May 24, 2023

BenWibking May 24, 2023

forrestglines May 24, 2023

pgrete May 24, 2023

BenWibking May 24, 2023

forrestglines May 24, 2023

BenWibking May 24, 2023

forrestglines commented May 24, 2023

forrestglines commented May 30, 2023

forrestglines commented Jun 1, 2023

forrestglines commented Jun 1, 2023 •

edited

Loading

pgrete left a comment

pgrete commented Jun 2, 2023

		const Real edot = cooling_table_obj.DeDt(eint, rho);
		const Real t_cool = eint / edot;

add derived fields to cluster pgen #63

add derived fields to cluster pgen #63

Conversation

BenWibking commented May 18, 2023 • edited Loading

forrestglines commented May 23, 2023

Choose a reason for hiding this comment

BenWibking May 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forrestglines commented May 24, 2023

forrestglines commented May 30, 2023

forrestglines commented Jun 1, 2023

forrestglines commented Jun 1, 2023 • edited Loading

pgrete left a comment

Choose a reason for hiding this comment

pgrete commented Jun 2, 2023

BenWibking commented May 18, 2023 •

edited

Loading

BenWibking May 23, 2023 •

edited

Loading

forrestglines commented Jun 1, 2023 •

edited

Loading