Clean up "make all" for 5.2 branch

[samsrabin]

Description of changes

Resolves pylint complaints for 5.2 branch (PR #2372) described in Issue #2418. Note, @ekluzek, I was unable to reproduce the system test failure you described in that issue.

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed:

• Resolves Get make all clean on the ctsm5.2 branch #2418

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any: make all is now clean.

[samsrabin]

@ekluzek The system test failure you described in #2418 seems to be related to your addition of clm6_0 physics in #2417. I think it'd be better if you made a task in that issue to resolve it. That way, this PR can close #2418.

[slevis-lmwg]

Wow, this looks like a nice cleanup @samsrabin, thank you!

Given that the changes feel extensive (at least to me), and that we are about to finish ctsm5.2, it's important to confirm that "make all" doesn't now break elsewhere and that clm_pymods still works. And then maybe run an example of

./gen_mksurfdata_namelist
./gen_mksurfdata_jobscript_single

and one of

./gen_mksurfdata_jobscript_multi

(no need to generate fsurdat files) to confirm that answers have not changed. I'm happy to meet if that helps.

[ekluzek]

@slevis-lmwg and @samsrabin. @slevis-lmwg I think you are right that we need to do more testing here. But, I'm not sure that @samsrabin should do all of it. He did volunteer his time for this and I appreciate his efforts. So I think @slevis-lmwg and I should do more testing. But, I'll leave it up to @samsrabin if you are up to do more testing for us...

[ekluzek]

Also note @slevis-lmwg there are python tests for the three main scripts that you give above. So we only need to maybe do some testing on something that wouldn't be caught with those tests.

I ran simple tests for above from the main tool in tools/mksurfdata_esmf:

./gen_mksurfdata_namelist --start-year 1850 --end-year 2000 --res 0.9x1.25
./gen_mksurfdata_build.sh
./gen_mksurfdata_jobscript_single --number-of-nodes 10 --tasks-per-node 128 --namelist-file surfdata_0.9x1.25_hist_1850_78pfts_c240314.namelist./gen_mksurfdata_jobscript_multi --number-of-nodes 10 --scenario global-present

And compared the results and they are the same other than the paths to where the CTSM clone is.

[ekluzek]

I reviewed the testing being done compared to what we actually create using the Makefile. The jobscript_single and jobscript_multi are sufficiently simple that there doesn't look to be anything else that needs to be tested for them. The namelist script is the one with the complexity. The things that need to be done there are: simple, potveg, vic, hist, and SSP. And those are being tested in the python tests, so I'd say we are all good here.

This is an example of having sufficient testing that we can refactor the code without having fear that it's being screwed up. That's one of the benefits of having good testing in place.

I did run clm_pymods for the baseline and this branch, and they compare the same. There is one test that initially failed in both, that we should look into, but it acted the same as the baseline.

MKSURFDATAESMF_P64x1.f10_f10_mg37.I1850Clm50BgcCrop.derecho_intel

[ekluzek]

Nice work. And quick work. I'm thinking you should show us your process here.

This is great. I have a few questions, but nothing important to change.

I checked the number of pylint disable statements and they are reasonably few. I do point out that there are only two "too many statements" now, which is a great reduction. Eventually we should shorten that, but that's such an improvement already.

[samsrabin]

Thanks for the reviews, y'all! I don't think I have time for additional testing, though. I'll let you handle that if that's alright. Feel free to go ahead and merge this when you think it's ready.

[ekluzek]

Awesome, thanks so much @samsrabin this was really great work and I'm glad to see it come in.