remove unnecessary EnforceLimits on old state #668
Merged
Conversation
This file contains 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
Immediately after a regrid, the old state `state_old_cc_` is uninitialized (and unused). `EnforceLimits` does not need to be called on it. This avoids spurious FPEs from trying to process the uninitialized or NaN values.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
psharda
approved these changes
Jul 7, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 11, 2024
### Description Enables signal handling and floating-point exception (FPE) traps for NANs when running the test suite, so it will stop and report an error when NANs are encountered. (This only works on CPUs.) In order for this to work, we change the signal handling settings back to AMReX defaults. We also have to suppress FPEs when importing `numpy` (see numpy/numpy#20504). By default, many compilers perform optimizations that assume that floating-point exceptions are disabled, and will often produce spurious FPEs for vectorized code (however, this does not affect the results in any way). There are compiler-specific options to disable this behavior for Intel and Clang. Note that `AMReX_FPE=ON` only sets the compiler option `-ftrapv` (or similar). We instead add the runtime options `amrex.fpe_trap_invalid=1`, `amrex.fpe_trap_zero=1`, and `amrex.fpe_trap_overflow=1` to the command-line arguments for each test. These options can also be added to the input file (or command line) to debug individual problems. **Depends on:** * #667 * #668 * #671 ### Related issues Partially resolves #556. ### 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:_ - [x] I have added a description (see above). - [x] I have added a link to any related issues see (see above). - [x] I have read the [Contributing Guide](https://github.com/quokka-astro/quokka/blob/development/CONTRIBUTING.md). - [ ] I have added tests for any new physics that this PR adds to the code. - [x] I have tested this PR on my local computer and all tests pass. - [x] I have manually triggered the GPU tests with the magic comment `/azp run`. - [x] I have requested a reviewer for this PR. --------- Co-authored-by: Piyush Sharda <34922596+psharda@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
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.
Description
Immediately after a regrid, the old state
state_old_cc_is uninitialized (and unused, because all of the times of the regridded levels are synchronized at that moment). Therefore,EnforceLimitsdoes not need to be called on it.Since the old state is unused, this PR doesn't affect the simulation evolution in any way. However, this avoids spurious FPEs from trying to process zero or NaN values in the old state array.
Related issues
Fixes #401.
Checklist
Before this pull request can be reviewed, all of these tasks should be completed. Denote completed tasks with an
xinside the square brackets[ ]in the Markdown source below:/azp run.