Skip to content

Geometry: Exceptions#5219

Merged
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
ax3l:fix-probdomain-throw
Mar 23, 2026
Merged

Geometry: Exceptions#5219
WeiqunZhang merged 1 commit intoAMReX-Codes:developmentfrom
ax3l:fix-probdomain-throw

Conversation

@ax3l
Copy link
Copy Markdown
Member

@ax3l ax3l commented Mar 23, 2026

Summary

We use AMReX interactively with amrex.throw_exception = true to be able to handle user errors or convergence issues and act on them.

In Geometry, some methods are defined noexcept which breaks exception propagation in ImpactX
BLAST-ImpactX/impactx#1348 and still aborts the process as a consequence.

This removes the noexcept where they are not guaranteed to be true at runtime. First seen in computeRoundoffDomain which is called from a couple of places.

Performance impact for this host-side code should be minimal / not a problem.

Additional background

Throwing an actual abort in a Python script will abort the whole interpreter, resetting all its state, terminating the process. That is a big problem in interactive usage, e.g., Jupyter Notebooks, but also for general C++ - Python and back error handling logic.

BLAST-ImpactX/impactx#1350

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

We use AMReX interactively with `amrex.throw_exception = true`
to be able to handle user errors or convergence issues and
act on them.

In Geometry, some methods are defined `noexcept` which breaks
exception propagation in ImpactX
BLAST-ImpactX/impactx#1348
and still aborts the process as a consequence.

This removes the `noexcept` where they are not guaranteed to
be true at runtime. First seen in `computeRoundoffDomain` which
is called from a couple of places.

Performance impact for this host-side code should be minimal /
not a problem.
@AlexanderSinn
Copy link
Copy Markdown
Member

I also find the assert in computeRoundoffDomain particularly annoying, as it has a very nondescript message, and I think it even sometimes triggers for perfectly valid geometries if one of the sides is at exactly zero. Annoyingly, the roundoff domain is not even used in our code.

@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Mar 23, 2026

I agree, documented and moved to #5221 to improve.

@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented Mar 23, 2026

After this PR is merged, I will send another large LLM-assisted PR #5222 after that one is merged that will try to fix the pattern in the whole codebase of AMReX.

Relevant Checks we could add in Clang-Tidy

  • bugprone-exception-escape: The primary check for this issue, it flags functions that should not throw exceptions but potentially do.
  • performance-noexcept-move-constructor / performance-noexcept-destructor: These checks focus on ensuring that special member functions that are implicitly or are generally expected to be noexcept do not throw, as this can inhibit compiler optimizations or cause incorrect behavior in standard library containers.
  • modernize-use-noexcept: This check helps transition older throw() dynamic exception specifications to the modern noexcept specifier, using noexcept(false) if the function can throw.

How to Fix the Warning

If clang-tidy flags a function marked noexcept that might throw, you have a few options:

  • Remove the noexcept specifier: If the function is genuinely intended to be potentially throwing, remove the noexcept specifier (making it implicitly noexcept(false) in C++11 and later).
  • Ensure the function logic cannot throw: Wrap throwing operations in try-catch blocks, use non-throwing alternatives (e.g., in C++17, you might return a std::string_view from a getter instead of a std::string which can allocate memory and throw std::bad_alloc), or ensure all called functions are also non-throwing.
  • Use noexcept(false): Explicitly mark the function as noexcept(false) to clearly indicate it may throw exceptions, while still satisfying certain guideline-following tools.

@ax3l ax3l mentioned this pull request Mar 23, 2026
7 tasks
@ax3l ax3l requested a review from AlexanderSinn March 23, 2026 18:59
@WeiqunZhang WeiqunZhang merged commit 4d09691 into AMReX-Codes:development Mar 23, 2026
74 checks passed
@ax3l ax3l deleted the fix-probdomain-throw branch March 23, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants