Skip to content

[WIP] Make nc3crop and nc3irrig private to pftconMod#4112

Draft
samsrabin wants to merge 18 commits into
ESCOMP:b4b-devfrom
samsrabin:allow-new-non-crop-pfts
Draft

[WIP] Make nc3crop and nc3irrig private to pftconMod#4112
samsrabin wants to merge 18 commits into
ESCOMP:b4b-devfrom
samsrabin:allow-new-non-crop-pfts

Conversation

@samsrabin

@samsrabin samsrabin commented Jul 8, 2026

Copy link
Copy Markdown
Member

WORK IN PROGRESS; SUBJECT TO FORCE PUSHES; DO NOT REVIEW

Description of changes

As part of our goal to make our code agnostic to the ordering of PFTs (see #21; #3388), this PR makes the nc3crop and nc3irrig variables (PFT indices for the generic crops) private to pftconMod.

Remaining work

  • Fix unit testing
  • Test
  • Remove check that num_pfts_possible_natural == nc3crop

Specific notes

Contributors other than yourself, if any: None

CTSM issues resolved or otherwise addressed, if any:

Any user interface changes (namelist or namelist defaults changes)? No

Testing planned or performed, if any:

  • aux_clm
  • ctsm_sci (changes to how some bits in the fire emissions code work)
  • fates (just to make sure some new variables I added aren't ever needed under FATES)

Requirements before merge:

  • The code in this PR branch builds with no errors.
  • The code in this PR branch runs with no errors. Briefly describe tested configuration(s):
  • This either (a) does not change answers, (b) it only changes answers at roundoff level, or (c) I have performed a scientific evaluation of the answer changes. Which?:
  • I have reviewed relevant parts of the CLM documentation Tech Note or User's Guide to determine if anything needs to be changed or added. If it does, describe:
  • This PR either (a) does not create a need to update the documentation or (b) includes required documentation updates (see guidelines for contributing documentation). Which?:

@samsrabin samsrabin self-assigned this Jul 8, 2026
@samsrabin samsrabin added the b4b bit-for-bit label Jul 9, 2026
@samsrabin samsrabin changed the title [WIP] Allow new non-crop pfts [WIP] Makes nc3crop and nc3irrig private to pftconMod Jul 9, 2026
@samsrabin samsrabin added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Jul 9, 2026
@samsrabin samsrabin changed the title [WIP] Makes nc3crop and nc3irrig private to pftconMod [WIP] Make nc3crop and nc3irrig private to pftconMod Jul 9, 2026
@wwieder

wwieder commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@adrifoster asked if adding an extral moss PFT is critical for the ABOVE project, as there are unused PFTs for the Arctic sites she's running (which include "tropical trees") and could be adopted for moss.

Long-term this is something we'll need, but it's a larger project.

@samsrabin would like to bring this PR in, as it's mostly done, which seems appropriate.

@samsrabin samsrabin force-pushed the allow-new-non-crop-pfts branch 5 times, most recently from ed55e78 to 81c7b3f Compare July 9, 2026 22:11
@samsrabin samsrabin force-pushed the allow-new-non-crop-pfts branch from 81c7b3f to 509d713 Compare July 10, 2026 15:52
@samsrabin samsrabin marked this pull request as draft July 10, 2026 15:58
if ( (n_pfts < maxveg) .and. (n_pfts < nc3crop) )then
write(iulog,*) ' n_pfts = ', n_pfts, ' maxveg = ', maxveg, ' nat_pft = ', nc3crop
if (n_pfts < num_pfts_possible_natural) then
write(iulog,*) ' n_pfts = ', n_pfts, ' num_pfts_possible_natural = ', num_pfts_possible_natural

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the change that I'm most unsure about. One thing was easy: The original could be simplified from

if ( (n_pfts < maxveg) .and. (n_pfts < nc3crop) )then

to

if (n_pfts < nc3crop) then

without issue, because nc3crop is always ≤ maxveg. But then, is n_pfts < num_pfts_possible_natural equivalent to n_pfts < nc3crop?

I will add a check in pftconMod for this during testing, which I will make sure to remove before merge.

@samsrabin samsrabin Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All changes in this file are from renaming num_cfts_possible to num_prognostic_cfts_possible, which more clearly describes its meaning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

b4b bit-for-bit next this should get some attention in the next week or two. Normally each Thursday SE meeting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants