Fix some MPI broadcast issues in DGLC and other data models - #429
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.
|
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. |
Katetc
left a comment
There was a problem hiding this comment.
Looks fine to me Bill! Thanks for fixing this issue!
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