add units to index_for_diagnostic_printout metadata to avoid ccpp_prebuild.py error

[grantfirl]

A one line change that avoids an error in ccpp_prebuild.py.

(Note that ccpp_prebuild.py errors out ungracefully during units checking if one of the units is empty.)

[climbfuji]

Do we need to fix the ccpp-framework behavior?

[grantfirl]

Do we need to fix the ccpp-framework behavior?

Not on my account. When this happens, I just go into ccpp_prebuild.py and add some output to find out which variable is causing the problem. I don't know if this is the same as capgen's behavior or not, but perhaps @JulieSchramm already has a test for this (what happens when one of the units attributes are empty)?

[grantfirl]

Shall I go ahead and merge this or wait for further approvals?

FOR OTHER REPO WATCHERS, THIS AFFECTS THE SCM ONLY. THERE WAS NO ERROR PRESENT FOR FV3/UFS.

[climbfuji]

Argh, I forgot the UFS part. @dustinswales can you pull this SCM-only change into your RRTMGP PR? I know it requires updating the cascade of submodule pointers one more time, but if you don't mind ... If not, no problem. Thanks!

[dustinswales]

@climbfuji
Not a problem. Done

[climbfuji]

@climbfuji
Not a problem. Done

Thanks a lot!

[climbfuji]

This PR has been pulled into #514 and will be merged as part of it - do not merge manually.

[grantfirl]

@climbfuji
Not a problem. Done

Yes, thanks @dustinswales .

[grantfirl]

This PR has been pulled into #514 and will be merged as part of it - do not merge manually.

OK, since NCAR/ccpp-scm#213 requires this to work, I'll wait to merge that too.

[climbfuji]

This PR has been pulled into #514 and will be merged as part of it - do not merge manually.

OK, since NCAR/gmtb-scm#213 requires this to work, I'll wait to merge that too.

Should be done on Thursday (tomorrow is a federal holiday).

[JulieSchramm]

capgen will catch a missing "units" attribute (the whole units line is missing)
capgen will not catch a units = (blank units attribute)
capgen will not catch a bogus units = banana

So type carefully!

[grantfirl]

capgen will catch a missing "units" attribute (the whole units line is missing)
capgen will not catch a units = (blank units attribute)
capgen will not catch a bogus units = banana

So type carefully!

Thanks, @JulieSchramm . IMO, it's less about "us" typing carefully and more about what to accept/reject when reviewing others contributions. Since capgen doesn't catch a (units = [blank]) attribute, does it also error out ungracefully when trying to compare a non-empty string to an empty one? If so, depending on what kind of status updates or logging verbosity is set, maybe capgen will provide enough information for a user to find the issue without too much trouble. With ccpp_prebuild, my problem was that it errored out telling me that it couldn't compare an empty string, but you had no idea which variable amongst the thousands was causing the error using the default (no/low) logging. I think that if you turned on DEBUG level of logging that it would have let you know which variable caused the problem, though, so it wasn't that big of a deal.

[gold2718]

Is units = really valid? Perhaps capgen should generate an error for this condition (in which case, please open an issue there).
And who says banana is not a valid unit?

[climbfuji]

Is units = really valid? Perhaps capgen should generate an error for this condition (in which case, please open an issue there).
And who says banana is not a valid unit?

See NCAR/ccpp-framework#336

banana is for sure a valid unit! In human history, a lot of bananas have been sold or traded per piece and not per weight) I suppose ;-)

[climbfuji]

This bugfix was merged as part of #514.