ABLASTR: Warn Manager State is Per AMReX Cycle - #7186
Merged
Conversation
ax3l
added
bug
ax3l
force-pushed
the
topic-warn-manager-lifetime
branch
from
August 26, 2026 16:31
5ca33ff to
ed2ee54
Compare
ax3l
changed the title
ax3l
force-pushed
the
topic-warn-manager-lifetime
branch
from
August 26, 2026 17:01
ed2ee54 to
80d9411
Compare
ax3l
added a commit
to ax3l/amrex
that referenced
this pull request
Aug 26, 2026
`amrex::Initialize` and `amrex::Finalize` drained their hook stacks with `top()()` followed by `pop()`. A hook that registers another one with `ExecOnInitialize`/`ExecOnFinalize` therefore left the *calling* hook on top of the stack: it ran a second time, and the following `pop()` silently discarded the newly registered hook without ever running it. Move the hook off the stack before calling it, so that a hook which registers another one runs exactly once and the newly registered hook runs as well. Found while making the ABLASTR warn manager state per AMReX cycle in BLAST-WarpX/warpx#7186: a warning recorded from a finalize hook resurrects the warn manager singleton and loses the teardown hook that would have released it again.
5 tasks
`WarnManager::GetInstance()` held the singleton in a function-local `static`, so its state lived for the whole process instead of for one AMReX initialize/finalize cycle. A process that runs more than one simulation - a parameter scan in Python, or the pytest unit tests - saw the second simulation start with the warnings of the first one still in the list, and with `always_warn_immediately` and `abort_on_warning_threshold` still set to whatever the first inputs deck had chosen. The instance now lives in a `std::unique_ptr` that is created on first use and released from an `amrex::ExecOnFinalize` hook, so every cycle starts with a clean warning list, with the default warning settings and with a freshly sampled `m_rank`. Tying the reset to `amrex::Finalize()` rather than to `WarpX::ResetInstance()` keeps ABLASTR independent of WarpX, and covers the warnings that are recorded outside the lifetime of a WarpX instance, such as the ones from `check_mpi_thread_level()`. The teardown must not itself record a warning. The mutex is not recursive, so the instance is moved out under the lock and destroyed after the lock is released; and `amrex::Finalize()` calls `top()()` before `pop()`, so a teardown hook that is registered while the stack drains is silently discarded and the state would leak into the next cycle anyway. Neither is reachable today - `~WarnManager` is defaulted, and `initialize_warning_manager()` registers the teardown ahead of every other finalize hook - so the second one is only documented in the code. This follows BLAST-WarpX#7142 and BLAST-WarpX#7143, which removed the equivalent process-lifetime state from the WarpX singleton and from the Python input buckets.
ax3l
force-pushed
the
topic-warn-manager-lifetime
branch
from
August 26, 2026 17:23
80d9411 to
91791c0
Compare
WeiqunZhang
pushed a commit
to AMReX-Codes/amrex
that referenced
this pull request
Aug 26, 2026
## Summary `amrex::Initialize` and `amrex::Finalize` drained their hook stacks with `top()()` followed by `pop()`. A hook that registers another one with `ExecOnInitialize`/`ExecOnFinalize` therefore left the *calling* hook on top of the stack: it ran a second time, and the following `pop()` silently discarded the newly registered hook without ever running it. Move the hook off the stack before calling it, so that a hook which registers another one runs exactly once and the newly registered hook runs as well. ## Additional background Found while making the ABLASTR warn manager state per AMReX cycle in BLAST-WarpX/warpx#7186: a warning recorded from a finalize hook resurrects the warn manager singleton and loses the teardown hook that would have released it again. ## Checklist The proposed changes: - [x] 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
lucafedeli88
requested changes
Aug 27, 2026
lucafedeli88
left a comment
lucafedeli88
left a comment
Member
There was a problem hiding this comment.
Thanks for this PR, @ax3l !
I would suggest updating the PR description, as it refers to an initial (more complex) implementation of this fix. Besides this, the PR is OK (I also think that this simpler implementation is better)!
Member
Author
|
Oh good point, let me fix that description. |
Member
Author
|
All thanks to @dpgrote for the clean implementation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WarnManageris a process-lifetime singleton, so the warnings it recorded outlivedWarpX::Finalize(). A process that runs more than one simulation (e.g., a parameter scan in Python, or the pytest unit tests) saw the second simulation report the warnings of the first one again.WarnManager::Clear()andablastr::warn_manager::WMClear(), backed bymsg_logger::Logger::clear(), called at the end ofWarpX::Finalize(). The singleton and its lifetime are unchanged.Details and test coverage
Details
An earlier version of this PR gave the singleton a per-AMReX-cycle lifetime instead: a
std::unique_ptrcreated on first use, released from anamrex::ExecOnFinalizehook and guarded by a mutex. That reset more state, but it had to work aroundamrex::Finalize()draining its finalize stack withtop()()beforepop(), which silently drops a hook that is registered while the stack is draining. Clearing the message list fromWarpX::Finalize()covers the case that actually bites and is much easier to reason about.The warning settings are not part of the reset.
always_warn_immediatelyis re-set on everyWarpXconstruction, becauseinitialize_warning_manager()always callsSetAlwaysWarnImmediately()with the parsed-or-default value.abort_on_warning_thresholdis only assigned whenwarpx.abort_on_warning_thresholdis present in the inputs, so a second simulation that omits it keeps the threshold of the first one.Test coverage
Source/ablastr/warn_manager/WarnManager.cppandSource/WarpX.cppcompile clean with the project warning flags.The behavior itself is not covered by a test on
developmentyet. The natural home is the pytest harness of #7144, which runs several simulations in one process: a test there should assert that the second simulation's warning list starts empty, otherwise this can regress silently.