Skip to content

fix NaN assertions that never fired due to IEEE 754 NaN comparison#1875

Merged
BenWibking merged 2 commits into
developmentfrom
chong/issue1857
May 8, 2026
Merged

fix NaN assertions that never fired due to IEEE 754 NaN comparison#1875
BenWibking merged 2 commits into
developmentfrom
chong/issue1857

Conversation

@chongchonghe

@chongchonghe chongchonghe commented May 8, 2026

Copy link
Copy Markdown
Contributor

Description

Fix debug assertions that silently passed even when variables held NaN values, due to incorrect use of relational operators with NAN.

In IEEE 754, NaN != NaN always evaluates to true, so AMREX_ASSERT(x != NAN) and AMREX_ASSERT_WITH_MESSAGE(x != NAN, ...) are no-ops — they never fire regardless of whether x is NaN. Two sites were affected:

  • RadSystem::ComputeRadPressure in src/radiation/radiation_system.hpp: four assertions on Fn, Tnx, Tny, Tnz that guard against uninitialized flux/pressure-tensor components.
  • problem_main in src/problems/ParticleSink/testParticleSink.cpp: one assertion that boost_vel_x was actually read from the input file.

Both sites are replaced with std::isfinite(value), which correctly returns false for NaN and infinity.

Related issues

Fixes #1857.

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.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@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 replaces incorrect != NAN comparisons with std::isfinite() checks in testParticleSink.cpp and radiation_system.hpp to properly handle NaN values. A review comment suggests using AMREX_ALWAYS_ASSERT_WITH_MESSAGE for input validation in testParticleSink.cpp to ensure the check remains active in release builds.

Comment thread src/problems/ParticleSink/testParticleSink.cpp Outdated
@chongchonghe chongchonghe marked this pull request as ready for review May 8, 2026 04:03
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. bug: wrong answer/failure/crash Something isn't working labels May 8, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 8, 2026
@BenWibking BenWibking added this pull request to the merge queue May 8, 2026
Merged via the queue into development with commit eb7a627 May 8, 2026
52 checks passed
@chongchonghe chongchonghe deleted the chong/issue1857 branch May 9, 2026 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug: wrong answer/failure/crash Something isn't working lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Radiation pressure debug assertions miss NaNs

2 participants