-
Notifications
You must be signed in to change notification settings - Fork 92
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
Facilitate crops in SP mode. #817
base: main
Are you sure you want to change the base?
Conversation
As you can see, there is still something up with the high arctic and the sahara, but that's likely nothing to do with crops... |
Thanks for your work on this @rosiealice ! I haven't gotten my head back into all of the discussions in #760 but can you clarify how much of #760 is done here? Two specific questions I have are: (1) Regarding this:
My understanding is that, at least eventually, we want crops still handled on their own landunits when running with FATES. Is that right? My understanding is also that, for non-FATES SP, CTSM puts crops on their own landunit (though I can never remember this for sure). (2) If this PR is supposed to fully address #760, can you please make some comments about how much testing and/or code reading you've done to verify the various things in #760 that were raised along the lines of "we might need to do X / we should look into whether X is needed". |
@billsacks to your question
The answer is yes. We have a namelist flag for this that we only allow FATES to set to .false. (create_crop_landunit). We actually want to remove the namelist option and hardwire it to true. |
@rosiealice per our conversion at the fates software meeting a couple mondays ago, I tested this branch out using a single pft with an f45 grid run. I'm still seeing the same failure mode from #782, so I think we need a separate fix for this issue. Results can be found on cheyenne here: |
Description:
To allow FATES-SP mode to generate crop areas, this PR updates the default PFT file to extend the map to HLM_PFTs 15 and 16. This means that for grid cells with crops, area is allocated. In SP mode, the HLM actually shifts the crops onto the natural vegetation land unit, hence the previous solution (running over crop land units as well as soil land units) does not in itself solve the problem.
Further, this PR also changes the order of operations logic related to the precision checking of the total patch area. Previously, this happened AFTER the call to init_cohorts, which appeared to result in cohorts that were larger than the patch area with an error the same size and that later trimmed off by the precision checking in EDInit (see changes). This PR makes the call to init_cohorts occur after the change in patch areas to account for precision-level errors in the inputs.
Adding the crop PFTs appeared to trigger this error. It is possible that this change will fix #782 which threw a similar error.
This PR addresses issue #760
Collaborators:
Expectation of Answer Changes:
yes, this will change answers in SP and NOCOMP modes.
Checklist:
Test Results:
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: