Skip to content

Skip radiation number density during timestep control#2004

Draft
James471 wants to merge 6 commits into
developmentfrom
james471/no-rad-conv
Draft

Skip radiation number density during timestep control#2004
James471 wants to merge 6 commits into
developmentfrom
james471/no-rad-conv

Conversation

@James471

@James471 James471 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Description

This PR skips radiation number density from convergence check in microphysics integration. There's no performance gain when tested on my laptop.
plot_mups

Checklist

Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an x inside the square brackets [ ] in the Markdown source below:

  • I have added a description (see above).
  • I have added a link to any related issues (if applicable; see above).
  • I have read the Contributing Guide.
  • I have added tests for any new physics that this PR adds to the code.
  • (For quokka-astro org members) I have manually triggered the GPU tests with the magic comment /azp run.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the SKIP_PHOTOCHEMFLUX compile-time option to optimize photochemistry calculations by skipping the tracking of radiation flux variables when they are not needed. It also configures several test problems to use the Rosenbrock integrator. The review feedback highlights two important issues: first, a potential division-by-zero or floating-point overflow in photochemistry.hpp when n_gamma_initial is extremely small, which can be mitigated by using a safer threshold; second, a missing update to the microphysics compilation setup in StromgrenSphereRSLA/CMakeLists.txt to enable the Rosenbrock integrator.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

const Real n_gamma_final = photochemstate.rn[0 + MicrophysicsNumRadVarsPerGroup * nn];
state(i, j, k, firstChemIndex + Physics_NumVars::numRadVarsPerGroup * nn) = n_gamma_final * chemBandQuanta[nn];
#ifdef SKIP_PHOTOCHEMFLUX
const Real flux_ratio = (n_gamma_initial[nn] > 0.0_rt) ? (n_gamma_final / n_gamma_initial[nn]) : 0.0_rt;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Using 0.0_rt as the threshold for n_gamma_initial[nn] can lead to floating-point overflow (to infinity) when n_gamma_initial[nn] is extremely small (e.g., subnormal values close to underflow) and n_gamma_final is clamped to small_x. If the initial flux is 0.0, multiplying it by infinity produces NaN, which will propagate and corrupt the simulation state. Using a safe, physically negligible threshold like 1e-100_rt prevents this overflow and subsequent NaN propagation.

const Real flux_ratio = (n_gamma_initial[nn] > 1e-100_rt) ? (n_gamma_final / n_gamma_initial[nn]) : 0.0_rt;


# this will add #define CHEMISTRY
target_compile_definitions(${JOB_NAME} PUBLIC PHOTOCHEMISTRY)
target_compile_definitions(${JOB_NAME} PUBLIC PHOTOCHEMISTRY SKIP_PHOTOCHEMFLUX SKIP_RAD_NUM_CONV)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Since StromgrenRSLA.toml has been updated to use integrator.rosenbrock_tableau = 3, the setup_target_for_microphysics_compilation call at the beginning of this file (around line 6) should also be updated to pass "Rosenbrock" as the third argument, similar to the changes made in DTypeFront and StromgrenSphere. Otherwise, the Rosenbrock integrator will not be compiled for this target, leading to build or runtime failures.

@James471

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 4 pipeline(s).

@James471

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 4 pipeline(s).

@BenWibking BenWibking mentioned this pull request Jun 26, 2026
5 tasks
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