Skip to content
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

Merged

Conversation

DusanJovic-NOAA
Copy link
Collaborator

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?

  • Will the code updates change regression test baseline? If yes, why? Please show the baseline directory below.
  • Please commit the regression test log files in your ufs-weather-model branch

Dependencies

N/A

Copy link
Collaborator

@junwang-noaa junwang-noaa left a 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?

@DusanJovic-NOAA
Copy link
Collaborator Author

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:

20210322.06Z.C96.64bit.non-mono
2021 03 22 06 0 0

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.

@junwang-noaa
Copy link
Collaborator

Yes, what I mean is that diag_table does not have any output fields.
Can we set dopost to false if all the fieldCount from history bundles are zero? Other than that, I think at this time it's the developers' job to use correct post control file if the history files do not contain enough model fields to produce post products.

@DusanJovic-NOAA
Copy link
Collaborator Author

Yes, what I mean is that diag_table does not have any output fields. Can we set dopost to false if all the fieldCount from history bundles are zero? Other than that, I think at this time it's the developers' job to use correct post control file if the history files do not contain enough model fields to produce post products.

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.

@junwang-noaa
Copy link
Collaborator

OK. I agree with you.
@WenMeng-NOAA just let you know that if developers do not include output fields in the diag_table, they need to turn off inline post (write_dopost: .false.). Model will not check inline post setting from the content in diag_table, it will make model code too messy.

@zach1221
Copy link
Collaborator

Good morning, @BrianCurtis-NOAA . Testing is complete on #1825 . Can you please merge this PR for us?

@BrianCurtis-NOAA BrianCurtis-NOAA merged commit 1158696 into NOAA-EMC:develop Jul 28, 2023
@DusanJovic-NOAA DusanJovic-NOAA deleted the empty_output_bundle branch August 31, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Model fails when output bundle contains no fields
4 participants