Collapse2gencrop branch: Allow the model to not need to read 16-pft datasets and rather read 78-pft datasets

[slevis-lmwg]

Description of changes

Erik converted his collapse2gencrop branch from svn to git. Sam cloned Erik's branch and updated to ctsm1.0.dev008. Resolved conflicts and ran this test successfully:
qcmd -- ./create_test ERP_D_P36x2_Ld3.f10_f10_musgs.I1850Clm50BgcCrop.cheyenne_intel.clm-default --compare ctsm1.0.dev008 --walltime 0:30 --project UIDA0003

Details documented here:
https://docs.google.com/document/d/1ey5DDYWail8T0Jy7X8K3Mo-cOaDtKTSoXQjVMrZADGI/edit?usp=sharing

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)?
No, except in special case of collapsing pfts and code for that case is in development.

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

Testing performed, if any:
qcmd -- ./create_test ERP_D_P36x2_Ld3.f10_f10_musgs.I1850Clm50BgcCrop.cheyenne_intel.clm-default --compare ctsm1.0.dev008 --walltime 0:30 --project UIDA0003

[ekluzek]

Next steps that I see would be:

• Make sure all standard testing works (cheyenne intel/gnu, hobart intel/gnu/nag)
• Make sure unit testing is working (also review the new tests I added and make sure it's complete, and if not add more unit tests for the new capability)
• Keep the branch updated to the latest ctsm tags on master
• Add some specific tests for the new capability to run SP mode from 79-pft surface and landuse.timeseries datasets.
• Run the above tests by hand and also add to the test suite
• Review everything make sure it all makes sense
• Look at opportunities to improve/cleanup the code that's changed
• Ask @billsacks and I to review it once it's both working and you think it's complete

[billsacks]

@slevisconsulting Thanks a lot for adding some new tests covering this behavior! However, what was your reason for making the two new tests based on the glcMEC_decrease testmod? Is that a fundamentally important part of this test? I'm thinking that these tests are orthogonal to glcMEC stuff, and so shouldn't be based off of that testmod.

If you do want these to be based off of the glcMEC_decrease testmod, then you should do this by having the include_user_mods file point to the glcMEC_decrease directory, and then only having the differences from that testmod in your new testmods directories. We try hard to avoid copying and pasting one testmod directory to another, by leveraging the include_user_mods mechanism.

I also noticed that you have some unused files in one of your new testmods directories. Can you please remove these? In git, you should try to avoid committing large files to the repo if they aren't really needed, because once they're in a commit, they're in history forever, bloating the repository. So my preference would be if you rewrote the last commit to remove these big new files (using git commit --amend followed by a force push... I can help you with that if you don't know how to do that).

As for the change of maxpatch_pft vs. numpft+1: I think @ekluzek can address this better than I can. However, if we do stick with numpft+1 in the places where you have changed it, then I'd request that a new variable be added in clm_varpar that is currently set to numpft+1 and is named with something meaningful, to make this code more clear and to make it easier to change this in the future (if numpft+1 is no longer the correct expression).

Finally, a note about commit messages: your commit messages will show up nicer if you wrap your lines to 72 characters. It's especially helpful if keep your first line to 72 characters or less. Sometimes it takes some work to summarize what you did succinctly in 72 characters or less, but this keeps things more readable, especially on github.

[slevis-lmwg]

Bill, I would not mind your help with the
git commit --amend ...
force push ...
sequence, keeping in mind that I already merged to the latest master since that commit and resolved corresponding conflicts and committed again. So I hope we can still make the correction.

I used the glcMEC_decrease test as a template to set up my new tests only because these were the first clm tests that I ever created. I will follow your advice and make the new tests distinct from glcMEC_decrease.

I agree about assigning numpft+1 to a variable. I will wait for confirmation from Erik before I proceed with that.

I think the 72-char issue arises because I often do
git commit -m 'text text text on and on'
and I think I can solve this by just typing git commit and using the editor.

[billsacks]

@slevisconsulting it is still possible to edit that commit, using git rebase -i (https://stackoverflow.com/questions/1186535/how-to-modify-a-specified-commit-in-git). If those instructions are unclear: If you push your latest version up to your fork, I'm happy to do the fix-up myself - which is probably easier for me than trying to coach you through it, since I haven't done this much before. (I can explain what I did after the fact.)

[slevis-lmwg]

Bill, I ended up with an error that seems like a can of worms to me, so I'm aborting the amend. This is what I tried:
git rebase --interactive 'b97f0fd'
In the editor I changed "pick" to "edit" for the "1b1e7b5" commit and saved and quit out of the editor.
git rm -r modified_paramfile_not_needed
git commit --all --amend --no-edit
git rebase --continue

error: could not apply dd9cd6d... added TracerConsistencyCheck; breakout of isotope lnd2atm/atm2lnd vars
When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply dd9cd6d... added TracerConsistencyCheck; breakout of isotope lnd2atm/atm2lnd vars

git rebase --abort

By the way 1: I forgot to mention that, in a later commit, I had already executed this command:
git rm -r modified_paramfile_not_needed
Does this mean that I should amend two commits? Add the files back to the commit where I removed them and then try removing them from the commit where I originally added them? I will not try this, yet, because I have now tried the next idea...

By the way 2: Here in github, I saw a trash icon and wondered whether I could simply click on that to delete the files. I tried this and it may have worked. However, my git push gets rejected now. I need to git pull first.

[billsacks]

@slevisconsulting I'm sorry to have sent you down the rabbit hole of git rebase --interactive. I realized that I did not give proper consideration to your caveat:

keeping in mind that I already merged to the latest master since that commit and resolved corresponding conflicts and committed again

which is what made this case difficult: I forgot that rebasing is difficult when you've already done a merge from master into your branch. I guess I'm still learning what's reasonably doable and what's a real pain in git, and I'm sorry to have dragged you along on that....

That said: Partly out of wanting to hone my git skills, and partly out of stubbornness, I have managed to accomplish the necessary rebase. The two keys were: (1) adding the --rebase-merges option to the git rebase -i command; and (2) even with that option, git forces you to redo the conflict resolution that you did when merging master into your branch (fortunately it was pretty easy in this case).

So if you're willing to go along with me just a bit more here, can you do the following so we can get a cleaner history? (I'm assuming here that you're in a git clone with collapse2gencrop checked out.)

(1) git remote add billsacks https://github.com/billsacks/ctsm.git

(2) git fetch billsacks

(3) git diff billsacks/collapse2gencrop

This should show no diffs; if there are any diffs, stop. (It probably means that you have committed locally something beyond what's pushed to your fork. But looking at your sandbox on cheyenne right now, you should be good here.)

(4) git status

The status should show that you have a clean sandbox before doing the following – i.e., no uncommitted changes. (Untracked files are okay.) (Again, looking at your sandbox on cheyenne right now, you should be good here.)

(5) git reset --hard billsacks/collapse2gencrop

This is a potentially destructive command, but it's safe here as long as (3) shows no diffs and (4) shows no uncommitted changes.

(6) git push slevisconsulting +collapse2gencrop

The + does a force push, which is necessary given the modified history.

[billsacks]

Oh, and as to this:

By the way 2: Here in github, I saw a trash icon and wondered whether I could simply click on that to delete the files. I tried this and it may have worked. However, my git push gets rejected now. I need to git pull first.

That is effectively the same as doing a git rm of the files followed by a git commit on the command line: it doesn't actually remove the files from history.

[slevis-lmwg]

Bill, I executed the list of commands, so we should be good.

[ekluzek]

@slevisconsulting I added a bunch of comments and questions. Since, you've done a lot of testing we don't necessarily need to do all of them. But, some could be done without redoing all of the testing. I also had some suggestions for the ChangeLog, and of course that can be done without redoing testing.

I also think you should change the name of this PR to just describe the change that's coming in, rather than a statement about it's history (which is already documented in the PR).

I also resolved a bunch of the comments, just so we don't think they are issues that need to be looked at.

[slevis-lmwg]

Thank you, @ekluzek! I will commit/push the easy stuff first. Then I will work on the last couple of comments still remaining.

[ekluzek]

Sam I committed a few changes to the ChangeLog, just seemed easier than trying to explain it. The bottom line is that it's not useful to have detailed notes on TPUTCOMP and MEMCOMP fails, since they aren't reliable or consistent for all tests. So a simpler answer is better here.

[slevis-lmwg]

Ok, thank you for doing that, Erik.

[slevis-lmwg]

@ekluzek I made the changes that you suggested, except that the if statement that you pointed out was removed a few commits ago, so you may be looking at an outdated version of that file.

[ekluzek]

OK, this looks good to go. @slevisconsulting I'm not sure you can do the next steps, because of permission. Do you see the "merge pull request" button above? If not I think everything is done, so I can do the next two steps which are to merge to master, and then create an annotated tag and push that to escomp. I'll also update the time-stamp in the Change files just before doing the merge and tag.

[billsacks]

@ekluzek - @slevisconsulting doesn't have write permission, so you should do this final step (see https://github.com/orgs/ESCOMP/teams/ctsm-write/members)