-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use FMS2 file_exists
, remove domain args
#1532
Use FMS2 file_exists
, remove domain args
#1532
Conversation
This patch uses the FMS2 `file_exists` function when using the FMS2 infra. Previously, the FMS1 version of this function was being used. This patch also removes the `mpp_domain` and `no_domain` arguments from the direct `file_exist` calls, which were not used by any known MOM6 configurations. (Nor would we expect them to be, since explicit references to FMS should not exist outside of the infra layer.) Since FMS2 does not use these arguments, their removal also creates a more meaningful interface between the two frameworks. Motivation: An issue with the FMS1 `file_exists` under the FMS2 infra was discovered in the UFS model on Hera. It was only reproducible in submitted jobs, and not for interactive jobs, and only with the GCC 9.2 compiler. (Other GCC versions were not tested.) One potential explanation is that it is related to the `save` attribute of the domain pointer, `d_ptr`. In the case above, `d_ptr` pointed to the MOM input domain for the failed cases. For the other working cases, `d_ptr` pointed to a `NULL()` value and behavior was normal. It is possible that `d_ptr` is inconsisently updated when FMS1 and FMS2 IO operations are used together, which should probably be considered undefined behavior.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1532 +/- ##
=========================================
Coverage 29.10% 29.10%
=========================================
Files 239 239
Lines 71524 71524
=========================================
Hits 20818 20818
Misses 50706 50706
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the direction of this change, but I would actually take this one step further. With the removal of the infrastructure-specific arguments from FMS_file_exists()
, I do not see any reason why FMS_file_exists()
and MOM_file_exists()
should be separate routines under the same overloaded interface. They could easily be merged by making the MOM_domain_type an optional argument to the combined file_exists()
routine.
The only potential snag that I see is that some compilers (PGI, I think) have difficulties handling renaming of a routine from a module use statement that has the same name original as a new interface (as is the case here with file_exists()
in the FMS2 version).
Let me think about your suggestion. My immediate preference is to keep using an interface, which is resolved at compile time, rather than a single function containing an There is also the issue that FMS1 will always go through the same I think this fallback to FMS1 is already a bit problematic, and perhaps we should look into how to do this for files with domains. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am convinced that this limited change is the right step, at least for now. This PR has passed TC testing and pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/13918.
This patch uses the FMS2
file_exists
function when using the FMS2infra. Previously, the FMS1 version of this function was being used.
This patch also removes the
mpp_domain
andno_domain
arguments fromthe direct
file_exist
calls, which were not used by any known MOM6configurations. (Nor would we expect them to be, since explicit
references to FMS should not exist outside of the infra layer.)
Since FMS2 does not use these arguments, their removal also creates a
more meaningful interface between the two frameworks.
Motivation:
An issue with the FMS1
file_exists
under the FMS2 infra was discoveredin the UFS model on Hera. It was only reproducible in submitted jobs,
and not for interactive jobs, and only with the GCC 9.2 compiler.
(Other GCC versions were not tested.)
One potential explanation is that it is related to the
save
attributeof the domain pointer,
d_ptr
. In the case above,d_ptr
pointed tothe MOM input domain for the failed cases. For the other working
cases,
d_ptr
pointed to aNULL()
value and behavior was normal.It is possible that
d_ptr
is inconsisently updated when FMS1 and FMS2IO operations are used together, which should probably be considered
undefined behavior.