-
Notifications
You must be signed in to change notification settings - Fork 77
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
Fix issue #293 NUOPC_CompAttributeIngest failure #316
base: develop
Are you sure you want to change the base?
Conversation
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.
This looks good to me. The way I interpret the new code, the use ends up with a single string value for both cases:
<COMP>_attributes::
restart_fh = 0.05,0.25,0.5,3,6
::
and
<COMP>_attributes::
restart_fh = 0.05, 0.25, 0.5, 3, 6
::
I think that is consistent behavior for how the non-HConfig Attribute handling is working string based.
9f0ba2d
to
b5d39e4
Compare
@theurich It does but note that
and
both return So although it works with whitespace now
returns I had to make a change because the NUOPC Comp UTest was failing. |
@tclune @NickSzapiro-NOAA This fix will concatenate all of the items with one space between each item, which is then added to the component's attributes. It's not without issues because it will trim all excess white space. See code for specific technical details. I did play around with ingesting multiple values as a valueList but there was little benefit. |
@danrosen25 I doubt this will affect GEOS, but if you want us to test a branch with GEOS, we'll be happy to oblige. Wish I could say we are entirely HConfig at this point, but production systems will be mostly Config for at least another year, not to mention legacy variants. |
Sorry - was assuming this was a change in the underlying ESMF_Config. GEOS itself does not use any NUOPC code, so this should only affect UFS. |
Hi @danrosen25. Sorry for the slow reply as I was on PTO. I'm not aware of any need to maintain white space. It's certainly better to be able to read the attribute (as string then parse) In the current NUOPC_refdoc, there are two:
If these are not implemented/supported, maybe this could be noted (only for the ESMF config format?) |
Fix #293
Append a token lists into single spaced string