runfv3: Fix exit code handling #1440
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
This bug was caused by python objects not being cleaned up before calling wrapper.cleanup. This bug caused the final timestep to not be saved in the regression data. Fixing it therefore changes the regression data. Resolves #1420
|
The changes in this PR confuse me. The regression test is configured to run for 30 minutes, with a 15-minute timestep. Since we save data at the end of each timestep, there should be 2 timesteps in the output data. @nbren12 |
|
Hmm. That is confusing. Seems like a possible bug with the python
diagnostics managers which was masked by the other bugs.
… |
1 task
spencerkclark
added a commit
that referenced
this pull request
Nov 8, 2023
… config (#2362) This PR is an initial piece broken out of #2350. It adds a `wrapper` attribute to the `UserConfig` class that is used to determine which Python wrapper to import within the `main.py` run file. As of this PR, the only supported option is the default `"fv3gfs.wrapper"`, but in the abstract this adds a hook for us to ultimately also support `"shield.wrapper"`. Added public API: - A `wrapper` can now be specified within a prognostic run `UserConfig`. The only currently supported option is the default of `"fv3gfs.wrapper"`. Because of this default, this PR is fully backwards compatible (no need to change already-defined configs). Significant internal changes: - The Python wrapper is now imported within the `main` function defined in `main.py` within the prognostic run instead of the top of the module. - The wrapper is initialized within the `main` function, and thus after all top-level imports. Initial tests (including with a 2x2 layout in the cloud) suggest the odd issue that motivated the previous approach may be resolved. Perhaps this is related to the fact that we no longer use OpenMPI in the prognostic run image (we use MPICH). This logic had originally been added in #706. - The garbage collection step added in #1440 has also been removed, since it appears it is no longer needed. - [x] Tests added Coverage reports (updated automatically):
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Resolves #1420. see commits for more details. This bug affected the regression data since it caused the final timestep to not be saved.