Fix some MPI broadcast issues in DGLC and other data models - #429
Open
billsacks wants to merge 3 commits into
Open
Fix some MPI broadcast issues in DGLC and other data models#429billsacks wants to merge 3 commits into
billsacks wants to merge 3 commits into
Conversation
Previously, the dglc restart read was only returning from the main task if the restart file didn't exist. Other tasks would proceed into the pio_openfile with a non-existent file. There were two specific problems behind this: - 'exists' (or its 'tmp' equivalent) was not being broadcast - The return for '.not. exists' was only done for the main task This commit fixes the logic to broadcast exists and return on all tasks if exists is false. This makes the logic more similar to what's in dshr_restart_read.
Note that the same issue of an unused mpicom argument exists in dshr_restart_read; I have not yet fixed it there because the changes would be more extensive.
Member
Author
|
There are some similar issues elsewhere in CDEPS that I am not fixing in this PR: (1) There are similar potentially problematic patterns in some other inquires in CDEPS (in dglc_comp_nuopc and dshr_mod) - where only the main task returns if a file doesn't exist, and so other tasks could potentially go on to try to read a non-existent file rather than exiting cleanly. I have left these as is because I'm not sure they're actually problems in practice, but we may want to fix them; if so, I feel that should be deferred to another PR (because I'm down too many rabbit holes already). (2) There are also some other unused mpicom arguments, but this would have required more extensive changes. |
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
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 of changes
This PR fixes a few issues related to broadcasts, mainly in DGLC, but one fix applies to all data models:
(1) In DGLC restart read, check file existence on all tasks
Previously, the dglc restart read was only returning from the main task
if the restart file didn't exist. Other tasks would proceed into the
pio_openfile with a non-existent file.
There were two specific problems behind this:
This PR fixes the logic to broadcast 'exists' and return on all tasks
if 'exists' is false. This makes the logic more similar to what's in
dshr_restart_read.
(2) Remove unused mpicom argument
(3) Fix broadcasts of restart file name to broadcast full file name
Previously, broadcasts of the restart file name incorrectly limited the number of characters broadcast. This one is fixed for all data models (in two places: one specific to DGLC and one in the data model share code).
Specific notes
Contributors other than yourself, if any: All of these issues were identified by Claude and fixed by a combination of Claude and myself. For any changes made by Claude, I have carefully reviewed them.
CDEPS Issues Fixed (include github issue #):
Are there dependencies on other component PRs (if so list):
Are changes expected to change answers (bfb, different to roundoff, more substantial): bfb
Any User Interface Changes (namelist or namelist defaults changes): no
Testing performed (e.g. aux_cdeps, CESM prealpha, etc):
Ran these three modified tests from the aux_cdeps test suite (the originals are SMS tests; changed to ERS to test the restart code):
Hashes used for testing: cesm3_0_beta09