Skip to content

Chong/test/vode no ngamma#1999

Draft
chongchonghe wants to merge 3 commits into
developmentfrom
chong/test/vode-no-Ngamma
Draft

Chong/test/vode no ngamma#1999
chongchonghe wants to merge 3 commits into
developmentfrom
chong/test/vode-no-Ngamma

Conversation

@chongchonghe

Copy link
Copy Markdown
Contributor

Description

Please write a short paragraph describing what changes this pull request makes to the code. It should provide sufficient context so that a reviewer can efficiently browse the source code changes.

Related issues

Are there any GitHub issues that are fixed by this pull request? Add a link to them here.

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.

@chongchonghe

Copy link
Copy Markdown
Contributor Author

/azp run rocm-quick

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@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 updates the Microphysics subproject commit and modifies the integrator configuration in inputs/DTypeFront.toml by setting integrator.atol_rad_num and integrator.radiation_failure_tolerance to 1e50, effectively disabling these tolerance checks. The feedback recommends updating the outdated inline comments for both parameters to accurately reflect that they are now disabled.

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.

Comment thread inputs/DTypeFront.toml
integrator.atol_spec = 1.4e-3 # cm^-3; 1e-5 × n_H = physical negligibility floor
integrator.atol_enuc = 1.24e8 # erg/g; c_v × 1 K temperature accuracy
integrator.atol_rad_num = 5e-2 # cm^-3; performance plateau for this problem
integrator.atol_rad_num = 1e50 # cm^-3; performance plateau for this problem

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.

medium

The comment # cm^-3; performance plateau for this problem is outdated and misleading now that integrator.atol_rad_num is set to 1e50 (which effectively disables the radiation tolerance check). Please update the comment to accurately reflect that this value is used to disable or bypass the tolerance check.

Suggested change
integrator.atol_rad_num = 1e50 # cm^-3; performance plateau for this problem
integrator.atol_rad_num = 1e50 # cm^-3; effectively disabled for this test

Comment thread inputs/DTypeFront.toml
integrator.atol_rad_num = 1e50 # cm^-3; performance plateau for this problem
integrator.species_failure_tolerance = 1.4e-3 # cm^-3; = atol_spec (1.5× final-state factor in vode_type.H absorbs BDF overshoot)
integrator.radiation_failure_tolerance = 0.5 # cm^-3; must exceed max N_gamma BDF overshoot at ionization front
integrator.radiation_failure_tolerance = 1e50 # cm^-3; must exceed max N_gamma BDF overshoot at ionization front

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.

medium

The comment # cm^-3; must exceed max N_gamma BDF overshoot at ionization front is outdated and misleading now that integrator.radiation_failure_tolerance is set to 1e50. Please update the comment to reflect that the failure tolerance is effectively disabled.

Suggested change
integrator.radiation_failure_tolerance = 1e50 # cm^-3; must exceed max N_gamma BDF overshoot at ionization front
integrator.radiation_failure_tolerance = 1e50 # cm^-3; effectively disabled for this test

@chongchonghe

Copy link
Copy Markdown
Contributor Author

/azp run rocm-quick

@azure-pipelines

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

@BenWibking

Copy link
Copy Markdown
Collaborator

Does this PR do the same thing as #2004?

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