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

change t-freeze calcualtion for sppt #10

Merged

Conversation

pjpegion
Copy link

@jiandewang these changes were tested with the data-atmosphere stochastic physics regression test, and passed.

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2022

Codecov Report

Merging #10 (80704a0) into dev-emc-candidate-20220824 (af12e89) will decrease coverage by 0.00%.
The diff coverage is 15.78%.

@@                      Coverage Diff                       @@
##           dev-emc-candidate-20220824      #10      +/-   ##
==============================================================
- Coverage                       37.22%   37.22%   -0.01%     
==============================================================
  Files                             261      261              
  Lines                           72319    72334      +15     
  Branches                        13533    13534       +1     
==============================================================
+ Hits                            26921    26924       +3     
- Misses                          40407    40419      +12     
  Partials                         4991     4991              
Impacted Files Coverage Δ
...parameterizations/vertical/MOM_diabatic_driver.F90 46.30% <15.78%> (-0.27%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jiandewang
Copy link
Owner

@pjpegion thanks for your quick work, can you combine your two commits log into one ? in general it is encouraged to use forced push to reduce minor commit (such as remove whitespace, et. al) to keep MOM6 log concise.

@pjpegion
Copy link
Author

How do I do that?

@jiandewang
Copy link
Owner

git reset --soft HEAD~2
git commit -m 'xxx'   
git push --force  ......
this will erase first two log, but use new commit message, still keep code change you made for the two commits

@pjpegion pjpegion force-pushed the ocn_sppt_review_fix branch from 80704a0 to 6ec8f5a Compare August 25, 2022 17:48
@pjpegion
Copy link
Author

Thanks, that seemed to work

@jiandewang
Copy link
Owner

I just tried your code on GAEA but stochastic job failed to match current baseline.
Do you have access to GAEA ? if not I will repeat that on ORION as HERA is fully saturated

@pjpegion
Copy link
Author

Yes, can you send me the path? I would expect the baseline to change, and was surprised that it passed on hera. I can take a look at the output to see if the changes are within my expectations

@jiandewang
Copy link
Owner

logfile : /lustre/f2/dev/ncep/Jiande.Wang/MOM6-update/Phil-T-freeze/ufs-weather-model/tests/log_gaea.intel/run_025_datm_cdeps_stochy_gefs.log

run dir: /lustre/f2/scratch/Jiande.Wang/FV3_RT/rt_38860/datm_cdeps_stochy_gefs

let me know if you have permission issue

@pjpegion
Copy link
Author

I cannot cd into /lustre/f2/scratch/Jiande.Wang.

@jiandewang
Copy link
Owner

can you try again ?

@jiandewang
Copy link
Owner

jiandewang commented Aug 25, 2022

failed to match BL in ORION, too

@pjpegion
Copy link
Author

I am puzzled. I see differences in the restart file, but they are in tropical waters that are above freezing. Also, the baseline on GAEA looks very different than the baseline on HERA, but the results from your regression test looks a lot like the baseline on hera. I'll like to take a look at your orion result too.

@jiandewang
Copy link
Owner

/work/noaa/stmp/jiwang/stmp/jiwang/FV3_RT/rt_26223/datm_cdeps_stochy_gefs
you may need to wait for 5 minutes for me to do the chmod

@jiandewang
Copy link
Owner

when you say difference in restart file, which variable are you referring ?

@pjpegion
Copy link
Author

I'm looking at temperature.

@jiandewang
Copy link
Owner

I gathered all restart file on HERA
/scratch1/NCEPDEV/climate/Jiande.Wang/working/MOM6-PHil-T-freeze/compare
but found HERA and ORION are identical

@pjpegion
Copy link
Author

I just realized I wasted a bunch of time. The regression test does not pass on hera either. I was fooled by the last line in the log file:

Thu Aug 25 15:33:04 UTC 2022
Start Regression test

Compile 001 elapsed time 188 seconds. -DAPP=NG-GODAS -DMPI=ON -DCMAKE_BUILD_TYPE=Release -DMOM6SOLO=ON

baseline dir = /scratch1/NCEPDEV/nems/emc.nemspara/RT/NEMSfv3gfs/develop-20220805/INTEL/datm_cdeps_stochy_gefs
working dir = /scratch2/BMC/gsienkf/Philip.Pegion/stmp2/Philip.Pegion/FV3_RT/rt_200128/datm_cdeps_stochy_gefs
Checking test 001 datm_cdeps_stochy_gefs results ....
Comparing RESTART/MOM.res.nc ............ALT CHECK......NOT OK
Comparing RESTART/iced.2011-10-02-00000.nc ............ALT CHECK......NOT OK
Comparing RESTART/DATM_GEFS_NEW.cpl.r.2011-10-02-00000.nc ............ALT CHECK......NOT OK

0: The total amount of wall time = 153.114783
0: The maximum resident set size (KB) = 962132

Test 001 datm_cdeps_stochy_gefs FAIL

REGRESSION TEST WAS SUCCESSFUL
Thu Aug 25 15:43:25 UTC 2022
Elapsed time: 00h:10m:23s. Have a nice day!

All three platforms have small coherent differences in areas where the water temperature is close to freezing, which is expected. The larger differences that I was seeing seems to be an instability in the model that grows over times, which is just two solutions diverging since there are small differences earlier in the forecast.

@jiandewang
Copy link
Owner

puzzled solved, I am going to do the merging.

@jiandewang jiandewang merged commit 2cd0cd3 into jiandewang:dev-emc-candidate-20220824 Aug 25, 2022
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