Closed eclare108213 closed 1 year ago
This looks fine to me. Is there a reason it's a Draft at this point? Do you want me to do any additional testing (with the caveat that cheyenne is down most of this week, so it might be a few days)?
I am satisfied with the testing that I've done so far, although I still plan to look more closely at the output differences. I compared base_suite and the QC -- are there other test suites that need to be run in this case?
I'd like to keep this as a draft for now, in case Nicole turns up something that needs to be changed.
Full test suite on cheyenne looks fine. The snw tests changed answers, but otherwise, all results pass and are bit-for-bit. This testing was done up to #2e12190. If you want me to test the latest version as well, let me know. Test results are here, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#993d058f5042a29d7d8b645313d9352632affa88.
Are the bulk changes not in this PR?
@dabail10 : No.
Full test suite of Icepack and CICE run on cheyenne. Plus a QC test. Everything looks good.
Icepack is fully bit-for-bit except for snw test cases, https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#8fe89cdbd3ed62f14baade3170bacdfb6d11cc60
CICE is bit-for-bit except for snw test cases, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#993d058f5042a29d7d8b645313d9352632affa88. In addition, 3 results are still pending, but those should be fine. Also, there is a problem with some part of the test scripts on cheyenne, but manual comparison of a few of those cases show the results are bit-for-bit. Will need to look into the test script problem.
The QC test passes,
INFO:main:Running QC test on the following directories: INFO:main: /glade/scratch/tcraig/CICE_RUNS/cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qcb INFO:main: /glade/scratch/tcraig/CICE_RUNS/cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qc INFO:main:Number of files: 1825 INFO:main:2 Stage Test Passed INFO:main:Quadratic Skill Test Passed for Northern Hemisphere INFO:main:Quadratic Skill Test Passed for Southern Hemisphere INFO:main:Creating map of the data (ice_thickness_cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qcb.png) INFO:main:Creating map of the data (ice_thickness_cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qc.png) INFO:main:Creating map of the data (ice_thickness_cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qcb_minus_cheyenne_intel_smoke_gx1_72x1_icdefault_medium_qc_snwgrain_snwitdrdg.qc.png) INFO:main: INFO:main:Quality Control Test PASSED
@eclare108213, looks like this could be merged. Then the CICE PR needs to be updated with this version of Icepack, and it can be merged. Are you able to do both quickly? I can do the CICE merge after a quick final review and after the Github actions runs. Maybe we can get both in before weekend testing. If not, that's OK too.
Draft PR while @njeffery tests the code in E3SM. There is still some code cleanup to do, too.
[x] Short (1 sentence) summary of your PR: Updates to advanced snow physics implementation from E3SM.
[x] Developer(s): @eclare108213 @njeffery
[x] Suggest PR reviewers from list in the column to the right.
[x] Please copy the PR test results link or provide a summary of testing completed below.
The answers change for the advanced snow physics tests due to changes in logic and a bug fix
_Icepack basesuite:
20 measured results of 20 total results 16 of 20 tests PASSED 0 of 20 tests PENDING 0 of 20 tests MISSING data 4 of 20 tests FAILED icepack/testsuite.test_snow_suite2> results.csh | grep FAIL FAIL chicoma_intel_smoke_col_1x1_run1year_snw30percent compare base_snow_suite2 different-data FAIL chicoma_intel_smoke_col_1x1_run1year_snwitdrdg compare base_snow_suite2 different-data FAIL chicoma_intel_smoke_col_1x1_run1year_snwgrain compare base_snow_suite2 different-data FAIL chicoma_intel_smoke_col_1x1_run1year_snwgrain_snwitdrdg compare base_snow_suite2 different-data
_CICE basesuite: The only changes to CICE other than the icepack submodule are the chicoma machine files. The two jra55do runs fail because I haven't yet downloaded the data.
cice.snow/testsuite.test_snow2> results.csh | grep FAIL FAIL chicoma_intel_smoke_gx3_8x2_jra55do run -1 -1 -1 FAIL chicoma_intel_smoke_gx3_8x2_jra55do test FAIL chicoma_intel_smoke_gx3_4x1_debug_icdefault_snwgrain_snwitdrdg complog base_snow different-data FAIL chicoma_intel_smoke_gx3_4x1_debug_icdefault_snwgrain_snwitdrdg compare base_snow 41.57 13.19 15.29 different-data FAIL chicoma_intel_smoke_gx3_4x1_debug_icdefault_snw30percent compare base_snow 41.13 13.36 14.66 different-data FAIL chicoma_intel_restart_gx3_8x2_icdefault_snwgrain_snwitdrdg complog base_snow different-data FAIL chicoma_intel_restart_gx3_8x2_icdefault_snwgrain_snwitdrdg compare base_snow 12.29 2.38 4.55 different-data FAIL - test failed 7 of 345 tests FAILED
CICE QC test:
INFO:main:Running QC test on the following directories: INFO:main: /lustre/scratch5/.mdt1/eclare/CICE_RUNS/chicoma_intel_smoke_gx1_44x1_medium_qc.qc_base INFO:main: /lustre/scratch5/.mdt1/eclare/CICE_RUNS/chicoma_intel_smoke_gx1_44x1_medium_qc.qc_test INFO:main:Number of files: 1825 INFO:main:2 Stage Test Passed INFO:main:Quadratic Skill Test Passed for Northern Hemisphere INFO:main:Quadratic Skill Test Passed for Southern Hemisphere
How much do the PR code changes differ from the unmodified code?
Does this PR create or have dependencies on CICE or any other models?
Does this PR add any new test cases?
Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
[x] Please provide any additional information or relevant details below:
Add chicoma machine files.
Use snwredist and snwgrain instead of tr_snow in column physics (tr_snow is still used in Icepack's driver but is not required for other drivers).
Correct tr_snow logic to use snwredist and snwgrain, as needed. This change is not BFB due to tracer values when the various options are NOT in use.
Fix snwgrain bug: the tracers smicen and smliqn were incorrectly calculated, missing the factor of aice needed to translate between grid-cell-mean thickness and "actual" ice thickness for the thermodynamics.