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

replace MOM_stochastics_stub with stochastic_physics stub #6

Conversation

pjpegion
Copy link

@pjpegion pjpegion commented Nov 5, 2021

@jiandewang I made the code changes the follows my understanding of the reviewer's comments. I couldn't push directly to your branch so here is a PR.
I tested this using the data-atmosphere regression tests, and both the control and stochastic physics tests pass.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021

Codecov Report

❗ No coverage uploaded for pull request base (EMC-stochastic-candidate-20211028@8e80a13). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head ea33065 differs from pull request most recent head 10526ff. Consider uploading reports for the commit 10526ff to get more accurate results
Impacted file tree graph

@@                         Coverage Diff                          @@
##             EMC-stochastic-candidate-20211028       #6   +/-   ##
====================================================================
  Coverage                                     ?   29.07%           
====================================================================
  Files                                        ?      239           
  Lines                                        ?    71740           
  Branches                                     ?        0           
====================================================================
  Hits                                         ?    20855           
  Misses                                       ?    50885           
  Partials                                     ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e80a13...10526ff. Read the comment docs.

@jiandewang
Copy link
Owner

jiandewang commented Nov 6, 2021

@pjpegion Thanks for the updating.
I just made a quick test in UFS but compiling failed in MOM_energetic_PBL.F90. Did you forget to commit some changes or maybe there is something I missed here ?
you can see one of the error on HERA at /scratch1/NCEPDEV/stmp2/Jiande.Wang/FV3_RT/rt_15755/compile_004/err around line 550.
My ufs working dir: /scratch2/NCEPDEV/climate/Jiande.Wang/working/Phil-stochastic/1105/UFS-test, here I used your latest MOM6 to replaced the current one.
Can you show me your related dir so that I can make a comparison ?

Phil:
The above failure is due to my bad, I forgot to modify mom6_files.cmake based on your code re-structure. Now my tests show datm-ocean compiling fine but S2S or S2SW still failed in compiling as stochastic_physics module is defined both in MOM6/config_src/external/stochastic_physics/ and stochastic_physics/stochastic_physics.F90, see error at
/scratch1/NCEPDEV/stmp2/Jiande.Wang/FV3_RT/rt_27927/compile_002/err

MOM6-interface/libmom6.a(stochastic_physics.F90.o):/scratch2/NCEPDEV/climate/Jiande.Wang/working/Phil-stochastic/1105/UFS-test-1/MOM6-interf
ace/MOM6/config_src/external/stochastic_physics/stochastic_physics.F90:4: first defined here
stochastic_physics/libstochastic_physics.a(stochastic_physics.F90.o): In function init_stochastic_physics': /scratch2/NCEPDEV/climate/Jiande.Wang/working/Phil-stochastic/1105/UFS-test-1/stochastic_physics/stochastic_physics.F90:213: multiple defini tion of stochastic_physics_mp_init_stochastic_physics_ocn
'_

code and script: /scratch2/NCEPDEV/climate/Jiande.Wang/working/Phil-stochastic/1105/UFS-test-1

@jiandewang
Copy link
Owner

update: for the total 10 of DATM jobs, 9 passed but datm_cdeps_stochy_gefs failed
FATAL from PE 3: call to init_stochastic_physics_ocn failed
see err at /scratch1/NCEPDEV/stmp2/Jiande.Wang/FV3_RT/rt_31002/datm_cdeps_stochy_gefs

@jiandewang
Copy link
Owner

can you share me your mom6_files.cmake ? Maybe I didn't modify it correctly since in your test stochastic run is OK

@pjpegion
Copy link
Author

pjpegion commented Nov 8, 2021

I did have to modify the mom6_files.cmake and CMakeLists.txt to accommodate the changes in MOM6. Please copy the files out of /scratch2/BMC/gsienkf/Philip.Pegion/ufs-weather-model.mom6pr/MOM6-interface and try again.
We will need to bring these changes into the ufs-weather-model when we merge these code changes back to EMC.

@jiandewang
Copy link
Owner

I modified mom6_files.cmake correctly but forgot to modify CMakeLists.txt.

For your test, if you go to the end of /scratch2/BMC/gsienkf/Philip.Pegion/ufs-weather-model.mom6pr/tests/log_hera.intel/run_datm_cdeps_stochy_gefs.log, it indicated the job failed.

For my new test, S2S failed in compiling, you can see log at
/scratch1/NCEPDEV/stmp2/Jiande.Wang/FV3_RT/rt_4572/compile_002/err

@pjpegion
Copy link
Author

pjpegion commented Nov 8, 2021 via email

@jiandewang
Copy link
Owner

you mean remove the following from mom6_files.cmake, right ?
MOM6/config_src/external/stochastic_physics/stochastic_physics.F90

@pjpegion
Copy link
Author

pjpegion commented Nov 8, 2021 via email

@jiandewang
Copy link
Owner

compiling fine now, let's wait a bit for the jobs to finish. Thanks

@jiandewang
Copy link
Owner

jobs run fine, I am going to merge your PR. In the meantime can you write a sentence to describe the code structure you made in my PR to GFDL at https://github.com/NOAA-GFDL/MOM6/pull/1538 ?

@jiandewang jiandewang merged commit e6bc901 into jiandewang:EMC-stochastic-candidate-20211028 Nov 8, 2021
jiandewang pushed a commit that referenced this pull request Feb 1, 2022
Add initialization tests using CS%initialized
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.

3 participants