NGEET / fates

repository for the Functionally Assembled Terrestrial Ecosystem Simulator (FATES)
Other
92 stars 89 forks source link

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

Open jenniferholm opened 1 month ago

jenniferholm commented 1 month ago

@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_cnpprescribed*? 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 commented 1 month ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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 https://github.com/ESCOMP/CTSM/pull/2336.

jenniferholm commented 4 weeks ago

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 commented 3 weeks ago

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

rgknox commented 1 week ago

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