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

Remove options not used, e.g. use_fates, use_cndv, and many more #46

Merged
merged 92 commits into from
Dec 9, 2022

Conversation

slevis-lmwg
Copy link
Collaborator

@slevis-lmwg slevis-lmwg commented Oct 13, 2022

Resolves #44 (I will continue updating my notes there for now)
Resolves #50

Collaborators: @ekluzek

Instead of rerunning the full SLIM test-suite, I have been confirming
that the following test continues to PASS, including against the
slim-n09_cesm2.1.4 baseline:
SMS.f19_g16.H_MML_2000_CAM6.cheyenne_intel.clm-global_uniform_g16_SOM
Testing: same as in previous commit
Same testing as for last two commits
Two tests failed in cheyenne test-suite (looks like same error to me):
SMS_D.f19_g17.IHistClm45BgcQianGs.cheyenne_gnu.clm-realistic_fromCLM5_1850
SMS_D_Lm1.f19_g17.I2000SlimRsGs.cheyenne_gnu.clm-2000_CMIP6_AMIP_1deg_ensemble
Both passed when I submitted separately.
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. It's hard to review these type of changes since they are so extensive. My main question is about the testing done.

We'll talk more at the standup this afternoon...

@ekluzek ekluzek marked this pull request as ready for review October 14, 2022 18:02
Cheyenne test-suite is currently running for this commit.
For the previous most recent commit, this test passed (no other
testing performed):
SMS_D.f19_g17.IHistClm45BgcQianGs.cheyenne_gnu.clm-realistic_fromCLM5_1850
SMS_D_Lm1.f19_g17.I2000SlimRsGs.izumi_nag.clm-2000_CMIP6_AMIP_1deg_ensembleMonthly
I will submit the full test-suite at the end of today.
…slim_slevis

Fix izumi gnu test-suite failure
- izumi gnu test-suite OK
- /bld cleanup mostly (if not entirely) NOT done
- pftconMod unchanged to avoid paramfile-related failure
SMS_Ld400.f19_g17.IHistSlim50QianGs.izumi_intel.clm-realistic_fromCLM5_1850Monthly
Rm orbital code that I missed in the previous commit
@ekluzek
Copy link
Collaborator

ekluzek commented Oct 26, 2022

With #50 I showed that it's safe to remove use_cn and BGC code.

@@ -106,31 +106,6 @@
<!-- Run restart tests for important ones, also do ERP for other compsets -->
<!-- Eventually we should also do ERI tests to make sure branch and hybrid work, but with #19 we can't -->
<!-- Run one longer test with production compiler for a few years to make sure longer science runs will continue to work -->
<!-- The important science simualtions for this configuration is the IHistClm45BgcQianGs case for Marysa -->
<test name="SMS_D" grid="f19_g17" compset="IHistClm45BgcQianGs" testmods="clm/realistic_fromCLM5_1850">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marysa are transient (eg. IHist) cases of much interest when using SLIM? I'm asking to determine whether we need extensive testing for transient cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekluzek wanted me to specify that the question pertains to I-cases, not F-cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this morning's SLIM stand-up, @marysa said that we may eventually use IHist with SLIM, so let's keep an IHist test in the test-suite.

<option name="comment">Debug smoke test for standalone SLIM</option>
</options>
</test>
<test name="PEM" grid="f19_g17" compset="IHistClm45BgcQianGs" testmods="clm/realistic_fromCLM5_1850">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep a PEM test but with I1850

@@ -141,65 +116,6 @@
<option name="comment">Longer smoke test for standalone SLIM, go over at least a year boundary (with clm5_0 and SP to ensure same as above)</option>
</options>
</test>
<test name="ERS_D_Ld60" grid="f19_g17" compset="IHistClm45BgcQianGs" testmods="clm/realistic_fromCLM5_1850Monthly">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep an ERS_D_Ld60 test but for I1850

<option name="comment">Longer smoke test for historical case for SLIM on izumi</option>
</options>
</test>
<test name="SMS" grid="f19_g17" compset="I1850Clm45BgcGs" testmods="clm/realistic_fromCLM5_1850">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep SMS for I1850 and remove for I2000

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these test changes should be pushed to the branch separately first, so that I generate new baselines.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and make a new tag at the same time

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekluzek
I tried to push the test changes that we discussed to the branch and got this error:
remote: Permission to ESCOMP/SimpleLand.git denied to slevisconsulting
Is this something that you can resolve?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually need @marysa to up some permissions. But, once she does that I'll be able to fix that. So you were pushing this part directly to the cesm2_1_slim branch rather than your cesm2_2_slim_slevis branch on your fork? There shouldn't be a problem pushing to your fork, but there would be to the cesm2_1_slim branch.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we've got this figured out @slevisconsulting. Go ahead and try again it should work now.

SMS_D_Lm1.f19_g17.I2000SlimRsGs.cheyenne_gnu.clm-2000_CMIP6_AMIP_1deg_ensembleMonthly
SMS_D_Lm1.f19_g17.I2000SlimRsGs.cheyenne_intel.clm-2000_CMIP6_AMIP_1deg_ensembleMonthly
Investigating...
Answers didn't change. Reason for going with coldstart:
The corresponding tests were aborting with variable not found in
initInterpMultilevelContainer.F90 lines 106-114, failing to find
coord_varname = 'COL_Z' and/or level_class_varname = 'LEVGRND_CLASS'.
Instead of reintroducing code that ultimately is unnecessary to SLIM
just to get this function to work, I decided to use coldstart for these
tests.
@slevis-lmwg slevis-lmwg self-assigned this Dec 9, 2022
@slevis-lmwg
Copy link
Collaborator Author

Most recent git ls-files | xargs wc -l gives 53288.

@slevis-lmwg slevis-lmwg merged commit 0f5a42e into ESCOMP:cesm2_1_slim Dec 9, 2022
@slevis-lmwg slevis-lmwg deleted the cesm2_1_slim_slevis branch December 9, 2022 20:00
@slevis-lmwg
Copy link
Collaborator Author

New baselines: slim-n11_cesm2.1.4
New tag: slim0.1.007_release-cesm2.1.4

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.

Remove use_cn and BGC compsets Remove use_fates, use_cndv, fates options, and other options not used
3 participants