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

Add back the cn30 params file and test for it #2843

Closed
slevis-lmwg opened this issue Oct 22, 2024 · 3 comments
Closed

Add back the cn30 params file and test for it #2843

slevis-lmwg opened this issue Oct 22, 2024 · 3 comments
Labels
bfb bit-for-bit

Comments

@slevis-lmwg
Copy link
Contributor

git grep -i cn30 returns
cime_config/testdefs/testmods_dirs/clm/clm60_monthly_matrixcn_soilCN30/user_nl_clm:paramfile = '$DIN_LOC_ROOT/lnd/clm2/paramdata/ctsm60_params_cn30.c240822.nc'
suggesting that we can remove this /testmods directory.

@slevis-lmwg slevis-lmwg added bfb bit-for-bit next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Oct 22, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Oct 23, 2024

Adding notes I put in the CTSM chat space:

That test came up in the context of testing for CN-Matrix where the matrix method failed for a high CN ratio. So we added a test to make sure that didn't break again. But, as @slevis-lmwg says we should make sure it's actually used in a test list and decide to either remove it or add it back in. In principle I think it is good to vary parameters in our testing to make sure the model works for the spectrum of values that might be used. But, I also wonder if this should be done varying more than just the one parameter, and maybe in a more systematic way...

The parameter file that goes with this just changes one parameter to 30 for a CN ratio. This is actually an example case where adopting the FATES on the fly conversion of the parameter file with the single change might make the most sense.

@slevis-lmwg
Copy link
Contributor Author

slevis-lmwg commented Oct 23, 2024

I like that idea.

Also, an update:
@olyson and I discussed this and decided that he will confirm the clm60...CN30 test works and will add it back to the testlist, as I have found no evidence that we removed this test intentionally. Keith is doing this as part of #2831.

@ekluzek ekluzek changed the title cn30 params file seems obsolete as the test that used it has been removed Add back the cn30 params file and test for it Oct 23, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Dec 5, 2024

Reviewing this today. This is done! Yay!

@ekluzek ekluzek closed this as completed Dec 5, 2024
@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit
Projects
None yet
Development

No branches or pull requests

3 participants