-
Notifications
You must be signed in to change notification settings - Fork 132
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
Move sw_redist, sw_frac, sw_dtemp from thermo to shortwave namelist #540
Conversation
Update verbose diagnostic output Update icepack to also include sw_redist, sw_frac, sw_dtemp changes in icepack driver (not required) Update version number in prep for next minor release
The test suite is complete and all tests pass. I am reviewing the model output now to confirm changes related to #534. |
I just pushed a new update. I spent a little time with the verbose output and did some cleanup, maybe I went a bit overboard. Mostly I just updated the spacing of the output. I attach sample output. Let me know if there are any comments. I think we can also change to "non-draft" now. I think it's ready for review and merge. |
Wow. This looks great. We should probably check the output for the tests where the changes are significant (not just changes to alignment), to make sure the logic came through correctly. It's hard to tell, looking at the code diff. |
If you look at this commit, 402a585, you'll see the diffs before my "cleanup". That represents most of the fixes outside formatting. |
endif | ||
write(nu_diag,1007) ' hfrazilmin = ', hfrazilmin,' minimum new frazil ice thickness' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so basically this makes the 'Radiation' section only show up if ktherm > 0
, right ? I agree with @eclare108213 that the diff is kind of hard to read, even in commit view...
It's a little easier on the command line, where we can tell Git to color moved lines in a different color, and also to detect moved lines even if indentation changes:
git show 402a585 --color-moved --color-moved-ws=allow-indentation-change
We can also ask to ignore changes in the amount of whitespace, this also helps:
git show 402a585 --ignore-space-change --color-moved --color-moved-ws=allow-indentation-change
I want to look at the output again before we release with this one. I'll submit a suite. |
I've done a sanity check on the verbose diagnostic output, skimming each of the cice.run files from base_suite. Looks good. Test results show 4 failures: |
PR checklist
Move sw_redist, sw_frac, sw_dtemp from thermo to shortwave namelist
apcraig
Full test suite run on cheyenne, all tests pass and are bit-for-bit. https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#402a58564017fcce82318982a3e09f92a654a2bc
Move sw_redist, sw_frac, sw_dtemp from thermo to shortwave namelist #537
Update verbose diagnostic output #534
Update icepack to also include sw_redist, sw_frac, sw_dtemp changes in icepack driver (not required)
Update version number in prep for next minor release