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

Added a Q10 for KmSx #12

Merged
merged 2 commits into from
Jan 19, 2022
Merged

Conversation

BernhardAhrens
Copy link
Contributor

Added one parameter to the DAMM function to make KmSx temperature-dependent with a Q10 function. Added additional slider to DAMMviz. However, I did not check the consequences for the other functions.

The range for the Q10 of KmSx is based on this paper:
Allison, S.D., Romero-Olivares, A.L., Lu, Y., Taylor, J.W., Treseder, K.K., 2018. Temperature sensitivities of extracellular enzyme Vmax and Km across thermal environments. Glob Chang Biol 24, 2884-2897.

I snuck in one additional change: when moisture is larger than porosity NaNs are returned

@AlexisRenchon
Copy link
Member

Hey @BernhardAhrens, thanks for the PR!
Are you familiar with Julia multiple dispatch?
The idea is that you can define the same function (e.g., DAMM()) multiple times, with different arguments types.
So, for instance, we could define DAMM(x::VecOrMat{<: Real}, p::NTuple{6, Float64}) and then DAMM(x::VecOrMat{<: Real}, p::NTuple{7, Float64}), and if a user give p with 6 params, it will use the original DAMM, if the user use p with 7 params, it will use the Q10Km DAMM.

I used multiple dispatch here, for example.

Do you want to try doing it? Otherwise I will just merge your PR and then make the changes myself.
Let me know!

@BernhardAhrens
Copy link
Contributor Author

I would let you go ahead with these changes. I think this can be a little bit of a design choice which I would leave up to you. I would rather have used a keyword argument with that Q10 set to 1.0 by default here for example

@AlexisRenchon
Copy link
Member

I agree Q10 = 1.0 is a better choice

@AlexisRenchon AlexisRenchon merged commit 12a15de into CUPofTEAproject:master Jan 19, 2022
@AlexisRenchon AlexisRenchon mentioned this pull request Jan 19, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants