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

Restart runs with stochastic physics due not reproduce #318

Closed
climbfuji opened this issue Dec 4, 2020 · 27 comments · Fixed by #668
Closed

Restart runs with stochastic physics due not reproduce #318

climbfuji opened this issue Dec 4, 2020 · 27 comments · Fixed by #668
Assignees
Labels
bug Something isn't working

Comments

@climbfuji
Copy link
Collaborator

Description

Runs with stochastic physics turned on do not give b4b identical results in restart runs. This is because the initialization logic does not support restart runs, and the necessary fields aren't written to the restart fields (sppt/shum/skeb/... weights, ...)

To Reproduce:

Use GFs v16beta regression test run directories to set up a 0->48h run, compare to a 0->24h + 24h->48h run. This does not reproduce. Turn off stochastic physics in all three runs to get b4b identical results (in PROD mode).

@climbfuji climbfuji added the bug Something isn't working label Dec 4, 2020
@pjpegion
Copy link
Collaborator

pjpegion commented Dec 4, 2020

Are you using the stochastic physics restart logic? In the original run, you need to set FHSTOCH=24 in this example. it will then dump out a restart for the stochastic physics called stoch_out.F<FHSTOCH>. And then for the restart run, you will rename stoch_out.F<FHSTOCH> to stoch_ini and set STOCHINI=.true. in the stochastic physics namelist block.

@climbfuji
Copy link
Collaborator Author

Are you using the stochastic physics restart logic? In the original run, you need to set FHSTOCH=24 in this example. it will then dump out a restart for the stochastic physics called stoch_out.F. And then for the restart run, you will rename stoch_out.F to stoch_ini and set STOCHINI=.true. in the stochastic physics namelist block.

No. This needs to be incorporated into the ufs restart capability, we unfortunately can't move files around and recompile for restart runs.

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 4, 2020

I'm confused, if you cannot move around files, how do the existing restart tests work? Maybe we should have a call.

@climbfuji
Copy link
Collaborator Author

I'm confused, if you cannot move around files, how do the existing restart tests work? Maybe we should have a call.

To conduct restart runs with the ufs-weather-model, one needs to update the forecast length in model_configure, flip 3-6 options in input.nml, and copy the NetCDF files written to RESTART/ to INPUT/. But no Fortran source code changes are required. Is stoch_out.F not a Fortran file?

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 4, 2020 via email

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 4, 2020

Now I see the source of the confusion. I did not escape the < and > signs around <FHSTOCH>. So it looked like a fortran file.
I have since edited my comment above.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Dec 4, 2020 via email

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Dec 4, 2020 via email

@climbfuji
Copy link
Collaborator Author

Ahhh ... got it. I like @SMoorthi-emc's suggestion for the name. As long as these are binary files, and we only need to rename those and update input.nml, that's fine.

This is definitely not included in any of the tests. I wonder if we could modify the ufs-weather-model logic (and possibly some code in stochastic_physics) that FHSTOCH will always be set to the the forecast length. Otherwise this will be totally confusing and lead to a lot of repeated runs. Restart files are written at the end of the model run by default, and the same should be true for stochastic physics in my opinion.

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 4, 2020

I like Moorthi's suggestion too,, I will just have to add logic in the initialization subroutine to make sure it has the forecast hour and can just read in the same named file. I will also have to coordinate with the DA workflow people since they are already handling the moving to stoch_out.F<FHSTOCH> to stoch_ini in the global_workflow.

Actually, I believe the logic that was added by Bing Fu does cause the restarts to be written out at the atmosphere's restart interval. But FHSTOCH needs to still be non zero in the namelist to trigger the restarting.

@climbfuji
Copy link
Collaborator Author

I like Moorthi's suggestion too,, I will just have to add logic in the initialization subroutine to make sure it has the forecast hour and can just read in the same named file. I will also have to coordinate with the DA workflow people since they are already handling the moving to stoch_out.F to stoch_ini in the global_workflow.

Actually, I believe the logic that was added by Bing Fu does cause the restarts to be written out at the atmosphere's restart interval. But FHSTOCH needs to still be non zero in the namelist to trigger the restarting.

We can update/overwrite FHSTOCH with the forecast length automatically. But, @junwang-noaa, isn't there a restart interval setting in model_configure that triggers the UFS to write out restart files at regular intervals? We need to support that if it exists.

@SMoorthi-emc
Copy link
Contributor

Dom, we need to have the ability to write intermediate restarts. For example, in the coupled model I specify the interval at which to write restart.
Some times hourly, some times 3 hourly, some time 15 days etc. We need this flexibility. It should also write restart at FHMAX, which should be the default.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Dec 5, 2020 via email

@climbfuji
Copy link
Collaborator Author

I agree with both of you - intermittent restarts and at the end of the run. @pjpegion please let me know if you need help implementing this.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Dec 5, 2020 via email

@climbfuji
Copy link
Collaborator Author

I know that. I was asking about stochastic physics. Dom seems to have suggested writing only at fhmax. Moorthi

Initially I did, but then I revised it saying that intermittent restarts are possible and need to be supported - so all good.

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 8, 2020 via email

@climbfuji
Copy link
Collaborator Author

Consistency is always good, but in the end the decision on the file names and file format is yours. I'd probably use consistent filenames but keep the format as-is in a first step, and in a second step switch to NetCDF. But, up to you!

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Dec 8, 2020 via email

@pjpegion
Copy link
Collaborator

pjpegion commented Dec 9, 2020

@climbfuji I want to try to call the stochastic_physics restart writing from module_fcst_grid_comp.F90, but I am having issues compiling. My issue is that the build system doesn't know about including the stochastic_physics directory as part of the include path at this point. How did you do it for FV3/stochastic_physics/stochastic_physics_wrapper.F90?

@climbfuji
Copy link
Collaborator Author

I added

target_include_directories(stochastic_physics_wrapper PRIVATE ${CMAKE_BINARY_DIR}/stochastic_physics)
target_include_directories(stochastic_physics_wrapper PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/mod>)
target_link_libraries(stochastic_physics_wrapper PUBLIC fms
                                                        stochastic_physics
                                                        gfsphysics
                                                        fv3dycore)

to stochastic_physics_wrapper/CMakeLists.txt. Not sure if you need the second line, try without. Also try to just add the stochastic_physics library to the exiting target_link_libraries if it is not there already.

@climbfuji
Copy link
Collaborator Author

Update: @pjpegion and I were able to figure this out.

I added

target_include_directories(stochastic_physics_wrapper PRIVATE ${CMAKE_BINARY_DIR}/stochastic_physics)
target_include_directories(stochastic_physics_wrapper PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/mod>)
target_link_libraries(stochastic_physics_wrapper PUBLIC fms
                                                        stochastic_physics
                                                        gfsphysics
                                                        fv3dycore)

to stochastic_physics_wrapper/CMakeLists.txt. Not sure if you need the second line, try without. Also try to just add the stochastic_physics library to the exiting target_link_libraries if it is not there already.

Update: @pjpegion and I were able to figure this out.

@junwang-noaa
Copy link
Collaborator

@climbfuji @pjpegion May I ask if the issue is fixed in Lisa PR #589?

@pjpegion
Copy link
Collaborator

This should be been fixed with my PR from March: #372

1 similar comment
@pjpegion
Copy link
Collaborator

This should be been fixed with my PR from March: #372

@climbfuji
Copy link
Collaborator Author

Do we have a test for it? Would be good.

@DeniseWorthen
Copy link
Collaborator

@pjpegion It looks like your PR #668 will add this restart test so I will link this issue.

@DeniseWorthen DeniseWorthen linked a pull request Jun 30, 2021 that will close this issue
14 tasks
epic-cicd-jenkins pushed a commit that referenced this issue Apr 17, 2023
…ates from release branch (#318)

* updated docs

* added git submodule

* fix formatting

* added new submodule commits

* fixed ref links

* finished Intro

* finish Components & Intro edits

* edited Rocoto workflow section of Quickstart

* added minor hpc submodule commits

* Updates to Rocoto Workflow in Quick Start

* add to HPC-stack intro

* submodule updates

* added submodule docs edits

* hpc-stack updates & formatting fixes

* hpc-stack intro edits

* bibtex attempted fix

* add hpc-stack module edits

* update sphinxcontrib version

* add .readthedocs.yaml file

* update .readthedocs.yaml file

* update .readthedocs.yaml file

* update conf.py

* updates .readthedocs.yaml with submodules

* updates .readthedocs.yaml with submodules

* submodule updates

* submodule updates

* minor Intro edits

* minor Intro edits

* minor Intro edits

* submodule updates

* fixed typos in QS

* QS updates

* QS updates

* QS updates

* updates to InputOutput and QS

* fix I/O doc typos

* pull updates to hpc-stack docs

* pull updates to hpc-stack docs

* fix table wrapping

* updates to QS for cloud

* fix QS export statements

* fix QS export statements

* QS edits on bind, config

* add bullet points to notes

* running without rocoto

* add HPC-Stack submodule w/docs

* split QS into container/non-container approaches

* added filepath changes for running in container on Orion, et al.

* edits to overview and container QS

* moved CodeReposAndDirs.rst info to the Introduction & deleted file

* continued edits to SRWAppOverview

* combine overview w/non-container docs

* finish merging non-container guide & SRWOverview, rename/remove files, update FAQ

* minor edits for Intro & QS

* updates to BuildRun doc through 3.8.1

* edits to Build/Run and Components

* remove .gitignore

* fix Ch 3 title, 4 supported platform levels note

* fix typos, add term links

* other minor fixes/suggestions implemented

* updated Intro based on feedback; changed SRW to SRW App throughout

* update comment to Intro citation

* add user-defined vertical levels to future work

* Add instructions for srw_common module load

* fix typo

* update Intro & BuildRunSRW based on Mark's feedback

* minor intro updates

* 1st round of jwolff's edits

* 2nd round of jwolff updates

* update QS intro

* fix minor physics details

* update citation and physics suite name

* add compute node allocation info to QS

* add authoritative hpc-stack docs to Intro

* create MacOS install/build instructions

* update HPC-Stack submodule/docs

* remove extra macinstall document

* revert hpc-stack submodule update

* update hpc-stack submod

* update w/release PR#282 changes

* update w/doc changes from PR#312

* clone develop

* Build/Run updates

* wcoss

* B/R check complete

* update UFS_UTILS links to latest

* update WM links to latest

* update ccpp/upp links to latest

* finish components update

* update config workflow chapter

* contrib guide updates

* update FAQ, fix formatting

* update Glossary

* update Graphics

* I/O updates

* intro updates

* update LAM grid chapter

* LAM grid update

* NCQS updates

* update WE2E

* rename quickstart files

* rename quickstart files

* add/update mac/linux modulefiles

Co-authored-by: gspetro <gillian.s.petro@gmail.com>
Co-authored-by: Natalie Perlin <68030316+natalie-perlin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants