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

Switching the default fates_cnp_prescribed_* parameters from 1 to 0 #1211

Closed
jenniferholm opened this issue May 30, 2024 · 6 comments · Fixed by #1136
Closed

Switching the default fates_cnp_prescribed_* parameters from 1 to 0 #1211

jenniferholm opened this issue May 30, 2024 · 6 comments · Fixed by #1136

Comments

@jenniferholm
Copy link
Contributor

@rgknox , Jing and I were chatting yesterday about CNP runs, and we think it makes sense to update the default "fates_cnp_prescribed_nuptake" and "fates_cnp_prescribed_puptake" from 1 (current default) to 0. Currently, when these parameters are set to 1, then there will be high levels of prescribed "supplemental" N and P.

This parameter is only used when CNP cycling is turned on, and parteh = 2, correct? So by default if users want to turn on CNP cycling they would like to have fully coupled nutrient simulations. Thus, 0=fully coupled simulation >0=prescribed (experimental). If users want to do something experimental, then they can manually change to >0.

Hi @rgknox - since the new arctic PFT update will mainly be a parameter file update, is that also a place where we can bring in new parameter file updates like this request for fates_cnp_prescribed_*? I think I remember you like to do a lot of parameter file change together in batches. Though that PR isn't even started yet.....

@glemieux
Copy link
Contributor

glemieux commented Jun 3, 2024

Per the fates software meeting today, @jenniferholm will create a pull request to address this and #1204 next week. We will combine the relevant future HLM-side API update PR with #1136 as well.

@rgknox
Copy link
Contributor

rgknox commented Jun 10, 2024

FWIW: I would like to deprecate the prescribed nutrient uptake capability. I added it in there to help test nutrient cycling through vegetation in fates while fates was not yet fully coupled to the HLM soil models. Since FATES nutrients are not fully coupled with CLM, prescribed uptake allows us to test the fates code with CLM still.

But this feature makes it more complicated for users, and has technical debt and code overhead associated with it. Once we have the acquisition module finished in CLM, I'd really like to get rid of prescribed uptake altogether.

@glemieux
Copy link
Contributor

Per the fates software discussion today, @rgknox noted that we will need to update the PRT2 regression test to build the parameter file on the fly to set the value back to 1. Currently we do something similar with seed dispersal and two stream regression testing, so we should be able to do the same with this in the corresponding API update PR. That said, we should revisit what is holding up ESCOMP/CTSM#2336.

@jenniferholm
Copy link
Contributor Author

Hi @rgknox - just so I'm aligned with next steps on what to do..... since you want to deprecate these 2 parameters anyway (once CLM and nutrients are coupled), should we not change the default setting from 1 to 0? Since you might still be using prescribed nutrients as a test in the CLM and FATES coupling and testing? So keep as is for now, then you will remove them later?

@rgknox
Copy link
Contributor

rgknox commented Jun 11, 2024

Since we don't have a timeline for getting the full nutrient cycling into FATES-CLM, lets go ahead and change the defaults.

@rgknox
Copy link
Contributor

rgknox commented Jun 27, 2024

Just issued this PR to CTSM: https://github.com/ESCOMP/CTSM/pull/2624/files

Once this is integrated, we are free to make this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants