add a new stream property 'gattr_update' to control whether to update global attributes #177
Conversation
… global attributes
guoqing-noaa
requested review from
clark-evans and
jderrico-noaa
as code owners
|
Hi @guoqing-noaa - thanks for this PR as well. I've manually requested the one failed test to re-run, as the output log indicated that it failed on one of its early wget commands (which is usually a hiccup on Github's end and not an issue with the code). The updated results should populate this PR soon. I noticed this appears to be related to PIO. Is PIO a requisite for rrfs-workflow or other applications? We have primarily used MPAS's SMIOL (simple MPAS I/O layer) and have little experience with PIO. |
|
The MPAS cmake build system requires that all output options be enabled while building. That means you are required to have PIO even if you don't want it. |
|
Thanks, @clark-evans! And thank @SamuelTrahanNOAA for input. In my impression, when we write out large MPAS output file, PIO will improve the performance. @SamuelTrahanNOAA Does your HFIP runs use PIO? |
We didn't set it up to use PIO, but perhaps Sam changed that later in the experiment? |
clark-evans
left a comment
clark-evans
left a comment
There was a problem hiding this comment.
Looks good to me pending one typo correction
src/framework/mpas_stream_manager.F
Outdated
| integer, intent(out), optional :: precisionProperty !< Output: Integer describing the precision of the stream | ||
| character (len=StrKIND), intent(out), optional :: filenameIntervalProperty !< Output: String containing the filename interval for the stream | ||
| integer, intent(out), optional :: clobberProperty !< Output: Interger describing the clobber mode of the stream | ||
| integer, intent(out), optional :: gattrUpdateProperty !< Output: Interger describing whether to update global attributes |
There was a problem hiding this comment.
Minor nitpick: "Interger" should be "Integer." I see NCAR's code also has this typo for clobberProperty, but I'm OK with leaving that one alone also correcting that line could cause us issues with future code merges.
There was a problem hiding this comment.
@clark-evans Thanks for finding out this! I will fix my part. Then leave the NCAR one as is?
There was a problem hiding this comment.
Yes, that's my recommendation - even as I'd love to update the NCAR part, I don't want to cause us merging issues later on.
dustinswales
left a comment
dustinswales
left a comment
There was a problem hiding this comment.
This doesn't break existing functionality. Thanks for adding this.
I do wonder if this is something that should make its way back to MPAS-Dev? (We don't need to do this now, but we have a label to keep track of these things)
Thanks @dustinswales |
dustinswales
added
the
upstream
|
Excellent @guoqing-noaa. |
|
@guoqing-noaa - please feel free to go ahead and update the version number to v8.3.1-2.5 in preparation for merging. Thanks! |
Done. Thanks, @clark-evans |
5 participants
When we do coldstart data assimilation on
init.ncusing JEDI, we will get a crash in MPAS IO with the following error:Per discussions with NCAR people and @SamuelTrahanNOAA, and the great debugging work by @SamuelTrahanNOAA, we identified that the problem came from
src/framework/mpas_stream_manager.Fwherempas_writeStreamAtttries to write not-defined global attributes toinit.nc. Since we don't expect JEDI to update any global attributes (at least forinit.nc), we can skip the relevantmpas_writeStreamAttpart when doing JEDI data assimilation.This PR is to address this issue by adding a new stream property
gattr_update. It takes values ofyesornoand defaults toyes. So it does not affect any existing MPAS-Model capability but allow coldstart JEDI DA to skip the relevantmpas_writeStreamAttpart to avoid crash, by settinggattr_update=noin theanalysisstream.Resolve issue #176
Priority Reviewers