-
Notifications
You must be signed in to change notification settings - Fork 157
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
Ignore empty output bundle #679
Ignore empty output bundle #679
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.
Thanks for the fix, the code changes look good to me. So now if quilt is on and diag_table is empty, the model will not crash, right?
diag_table file can not be empty (like zero size), it must have at least these two lines at the beginning, for example:
but it does not need to have any 'files' or 'fields' specifications. Also, post must be turned off, unless we add some test to detect that both dyn and phy bundles are not empty, and that both dyn and phy bundles have all necessary fields that post needs. But such a list does not exist, so I do not know how we can check that all post fields are present in both bundles. |
Yes, what I mean is that diag_table does not have any output fields. |
Where should we add such a test? Post gets the fields from output bundles in set_postvars_fv3 routine based on which grid_id it is processing at the moment and whether the bundle is a 'restart' bundle or non-restart bundle. Checking the number of fields in set_postvars_fv3 is probably too late. Adding the check somewhere in the write grid component will require non-trivial logic that loops over all bundles, skip all restart bundles, skip all native grid bundles (because post can nor read native grid fields), and finally skip all bundles on grids (domains) other than one currently being processed. I'm not sure adding all this logic is worth it. Especially since the criteria whether a bundle is empty or not is not enough to guarantee that the post will have access to all required fields. |
OK. I agree with you. |
Good morning, @BrianCurtis-NOAA . Testing is complete on #1825 . Can you please merge this PR for us? |
Description
(Instructions: this, and all subsequent sections of text should be removed and filled in as appropriate.)
Fix model crash if one or the output bundles does not have any field.
What bug does it fix, or what feature does it add? #676
Is a change of answers expected from this PR? No
Issue(s) addressed
Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)
Testing
How were these changes tested?
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?
Dependencies
N/A