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

Soil turnover times on parameter file and hard coded? #995

Closed
wwieder opened this issue Apr 26, 2020 · 7 comments · Fixed by #1039
Closed

Soil turnover times on parameter file and hard coded? #995

wwieder opened this issue Apr 26, 2020 · 7 comments · Fixed by #1039
Labels
bug something is working incorrectly

Comments

@wwieder
Copy link
Contributor

wwieder commented Apr 26, 2020

Here it looks like the soil turnover times are being read from the parameter file in DecompCascadeBGCreadNML

but then they are hard coded in decomp_rate_constants_bgc

! the belowground parameters from century

Values here look the same, but this may complicated the PPE @dlawrenncar wants to do?

@wwieder
Copy link
Contributor Author

wwieder commented Apr 27, 2020

Similarly for nitrificationk_nitr_max is on the parameter file, which limits nitrification rates (per second), but as far as I can tell, the code actually uses k_nitr_max_perday, which is a namelist variable.

@billsacks billsacks added the bug something is working incorrectly label Apr 27, 2020
@billsacks
Copy link
Member

I agree that it looks like k_nitr_max is never actually read from the netcdf file.

Regarding your first comment, I see this, which seems to explain the history of it:

! Todo: FIX(SPM,032414) - the explicit divide gives different results than when that
! value is placed in the parameters netcdf file. To get bfb, keep the
! divide in source.
!tau_l1 = params_inst%tau_l1_bgc
!tau_l2_l3 = params_inst%tau_l2_l3_bgc
!tau_s1 = params_inst%tau_s1_bgc
!tau_s2 = params_inst%tau_s2_bgc
!tau_s3 = params_inst%tau_s3_bgc
!set turnover rate of coarse woody debris
!tau_cwd = params_inst%tau_cwd_bgc

@wwieder
Copy link
Contributor Author

wwieder commented Apr 27, 2020

A few questions then as to the most efficient / appropriate way to:

  1. Remove legacy parameters from the parameter file that are not used (k_nitr_max)

  2. Replace the hard coded parameters with namelist or parameter file values that are actually being used (e.g. tau_s1 should be a namelist parameter = 1/18.5 (to avoid roundoff errors)).

Does this happen on @olyson's parameter branch? What about keeping track of namelist vs. parameter files changes / duplications?

@olyson
Copy link
Contributor

olyson commented Apr 28, 2020

Regarding k_nitr_max_perday, it looks like a namelist group, nitrif_inparm, was created at some point to hold this parameter and other parameters, but it is empty. So the model is using the hardcoded value of k_nitr_max_perday (0.1). The other parameters in this group are also using hardcoded values: denitrif_respiration_coefficient, denitrif_respiration_exponent, denitrif_nitrateconc_coefficient, denitrif_nitrateconc_exponent. And then these values are assigned to the params_inst% versions of these parameters.
So maybe we should populate the nitrif_inparm namelist group with these parameters and have the code use these. That should be bfb.

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 28, 2020

We've gone back and forth on the philosophy of if we should put everything into the namelist or on the params file. I'm guessing this namelist was added when we were thinking everything should go to namelist, but wasn't completed when we decided things should go on the params file.

At this point we actually think that things in namelist should go on the params file. At least longer term. So if you put it in namelist we will eventually want to move it to the params file. However, this is also true of other namelist parameters, so it will need to be done elsewhere. So if it's easier for you to do that now on your branch it still might be a reasonable solution.

@dlawrenncar
Copy link
Contributor

dlawrenncar commented Apr 28, 2020 via email

@ekluzek
Copy link
Collaborator

ekluzek commented Jun 11, 2020

Looks like this is essentially #138 and should be fixed with the hardcodep branch coming to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants