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

release/public-v2: b4b reproducibility for restart runs #246

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Feb 11, 2021

Description

This PR and associated PRs below fix the b4b reproducibility issues for the release/public-v2 branches. With these changes, restart runs are b4b identical to continues runs for regional and global applications with both GFS v15p2 and RRFS v1 alpha physics.

Changes:

Testing

See ufs-community/ufs-weather-model#417

Dependencies

NCAR/ccpp-physics#519
#246
ufs-community/ufs-weather-model#417

@climbfuji climbfuji marked this pull request as ready for review February 12, 2021 22:11
@climbfuji climbfuji changed the title WORK IN PROGRESS release/public-v2: b4b reproducibility for restart runs release/public-v2: b4b reproducibility for restart runs Feb 12, 2021
Copy link
Contributor

@SMoorthi-emc SMoorthi-emc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, it is ok.

Sfcprop(nb)%fice(ix) = zero
if (Sfcprop(nb)%slmsk(ix) == 2) Sfcprop(nb)%slmsk(ix) = 0
endif
! if (Sfcprop(nb)%fice(ix) < Model%min_lakeice) then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask if these statements commented out are required for b4 restart reproducibility? I am curious why this change is required as it might indicate the fice could be <min_lakeice in restart run.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this question is for Dom.
(this code in my branch is slightly different; it never made it to the dev version).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied over the exact changes that were made in PRs #178 and #212 for develop. I don't think we should try to do something different here than what was done for develop (and what took us - well, actually Moorthi - months to figure out when we finalized the GFS v16 prod equivalent in develop). Please see these two PRs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this question is for Dom.
(this code in my branch is slightly different; it never made it to the dev version).

Forgot to say, the code changes in this PR are from you originally - as you said, your slightly different version never made it to develop at that time, I remember those back and forth conversations quite well.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Feb 13, 2021 via email

@climbfuji
Copy link
Collaborator Author

Dom, I think some of this code is from Shan as some of my changes did not make in. Moorthi

Thanks for the clarification. Nonetheless, I think these changes are good and they definitely solve the problem with the b4b restart reproducibility (the global GFS v15p2 suite runs in release/public-v2 do not give b4b identical results in restart runs without this PR).

@climbfuji
Copy link
Collaborator Author

This PR is ready to merge.

@junwang-noaa junwang-noaa merged commit bbd81f5 into NOAA-EMC:release/public-v2 Feb 16, 2021
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.

4 participants