Skip to content

Add Johnson and Berry 2021 electron transport model option #1350

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

JessicaNeedham
Copy link
Contributor

Description:

This PR adds the Johnson and Berry 2021 model of electron transport response to irradiance as an option. Julien Lamour et al. have found this formulation has a better goodness of fit to gas exchange than the original FvCB model. See issue #1337 for details and discussion.

Note this needs to come in after #1262 . Parameter file and namelist option changes came in with #1344 . This requires the parameter file to namelist migration on the HLM side. Right now I am testing with this branch which has the new electron transport model switch but none of the other new ones.

The current configuration compiles and runs with the JohnsonBerry2021 namelist option switched on, but I haven't evaluated it yet. I'm still working on adding a history variable to track photosynthesis limitation by Rubisco and RuBP.

Collaborators:

@alistairrogers @JulienLamour @rgknox @rosiealice @glemieux

Expectation of Answer Changes:

With the FvCB1980 namelist option this should be b4b.
With the new JohnsonBerry namelist option we expect lower photosynthesis at intermediate irradiance.

Checklist

All checklist items must be checked to enable merging this pull request:

Contributor

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Documentation

Test Results:

Compiles but no formal testing yet.

@JessicaNeedham JessicaNeedham added the status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Mar 5, 2025
@rgknox
Copy link
Contributor

rgknox commented Mar 5, 2025

This is based on branch leaf_funcunit_tests, here is the diff until the former is integrated:

rgknox/fates@leaf_funcunit_tests...JessicaNeedham:fates:jb-electron

@JessicaNeedham
Copy link
Contributor Author

Chatting with @rgknox yesterday about how best to track how much photosynthesis is Rubisco or RuBP limited. It is possible but would require allocating memory to large arrays. An alternative is to use unit tests in python. This would call the photosynthesis code in isolation and would allow us to test under which conditions photosynthesis is Rubisco or RuBP limited using each electron transport model. @JulienLamour and @alistairrogers - do you think the unit test approach would work for you or is this something we should make a FATES history variable.

@JessicaNeedham
Copy link
Contributor Author

Screenshot 2025-03-07 at 10 38 41

This is the mean GPP over the last 10 years of a 20 year SP mode run on a 4x5 grid. Looks like GPP is ~5-10% lower in parts of the tropics with the new JB model.

Thanks @adrifoster for the SP mode parameter file! ☺️

@JessicaNeedham JessicaNeedham removed the status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Mar 7, 2025
@alistairrogers
Copy link

@JessicaNeedham this is great progress thanks! Is it possible to output global GPP (Gt yr-1) with the two model formulations?

@JessicaNeedham
Copy link
Contributor Author

@JessicaNeedham this is great progress thanks! Is it possible to output global GPP (Gt yr-1) with the two model formulations?

I get 105 Pg C yr-1 for the FvCB model and 97 PgC yr-1 for the JB model. Note that this isn't the final calibrated parameter file so it might be worth focusing more on the relative difference.

@alistairrogers
Copy link

Thanks, yes that was my goal.

@glemieux glemieux moved this from Under Review to Finding Reviewers in FATES Pull Request Planning and Status Mar 17, 2025
@glemieux
Copy link
Contributor

glemieux commented Apr 9, 2025

Regression testing is underway on derecho.

@rgknox
Copy link
Contributor

rgknox commented Apr 10, 2025

Submitted updates to @JessicaNeedham here: hold testing until reviewed and possibly integrated: JessicaNeedham#6

Removing unused global switches
@JulienLamour
Copy link

Thanks, it is awesome to see the impact of changing the model at a global scale!

I have checked the equation that was implemented for the JB electron transport model and did not see issues, but the conversion from Jmax to CB6fmax looks incorrect (https://github.com/JessicaNeedham/fates/blob/jb-electron/biogeophys/LeafBiophysicsMod.F90).

Equation 1

cb6fmax = (Qsat * jmax) / &
(Qsat * eta**-1.0_r8 - (jmax / phi) )

This should be something like:

Equation 2

jsat = GetJe_FvCB(Qsat,jmax,fnps)
cb6fmax = (Qsat * jsat) / &
(Qsat * eta**-1.0_r8 - (jsat / phi))

I am not familiar with fortran, so the code for Equation 2 may be wrong. The issue I see is to use Jmax in Equation 1, whereas it should be J simulated with the FvCB equation at Qsat.
Jsat can't be assumed to equal Jmax, there is a difference between the two of at least 10% that depends on Qsat and theta_psii.

One thing to discuss is the value of Qsat. I see that Qsat = 1275 was used.
To define Qsat, the most rigorous way would be to know the studies that were used to parameterize Vcmax and Jmax in Fates. That way, we could check the values of Qsat that were used to perform the A-Ci measurements.
If we can't find the references, we can assume that they use the same values we do, i.e., 1800. At least, this is the value we used in the tropics, and close to the value Alistair used in the Arctic (2000).

Thanks!

@glemieux glemieux moved this from Stuck to Under Review in FATES Pull Request Planning and Status Apr 23, 2025
@glemieux glemieux moved this from Under Review to Stuck in FATES Pull Request Planning and Status Apr 23, 2025
@alistairrogers
Copy link

Thanks for taking a look Julien. re Qsat I believe FATES is provides Qsat on an absorbed basis therefore I suggested 1275 assuming 1500 * 0.85 (common assumption for absorptance). I agree re knowing the incident irradiance of the measurements but for LSMs this is largely unknown. Perhaps we could use the median incident irradiance from the light response curves we analysed?

@glemieux glemieux moved this from Stuck to Final Testing in FATES Pull Request Planning and Status Apr 23, 2025
@glemieux glemieux moved this from Final Testing to Under Review in FATES Pull Request Planning and Status Apr 23, 2025
@glemieux glemieux moved this from Under Review to Hold in FATES Pull Request Planning and Status Apr 23, 2025
@glemieux glemieux moved this from Hold to Under Review in FATES Pull Request Planning and Status Apr 23, 2025
@JulienLamour
Copy link

JulienLamour commented Apr 24, 2025

Ok, I understand why 1275 was chosen. Choosing a small value for Qsat should lead to the lowest difference in GPP between models. So, this choice is conservative and makes sense for the comparison. If we can also do a run at 1530, this will give the range of variation of the GPP differences that we can expect.

A median incident irradiance of 1800 was used in the A-Ci curves (excluding crops not grown outside), but much of the data comes from our studies.

@alistairrogers
Copy link

@JessicaNeedham please could you confirm that Qsat is indeed absorbed irradiance. If so could you try a run with a Qsat of 1530 (incident irradiance 1800 *0.85 - typical assumption and irradiance used in deriving Jmax).

@JessicaNeedham
Copy link
Contributor Author

Not sure I understand the question. The way it is implemented currently, Qsat is defined as a local constant that we are assuming to be absorbed irradiance. It is not used elsewhere in the code.

I can start a run with Qsat = 1530.

@JulienLamour @alistairrogers - do I need to change the way cb6fmax is calculated to @JulienLamour 's suggestion above so that it uses Jsat calculated from the FvCB model instead of Jmax?

@alistairrogers
Copy link

sorry for the confusion @JessicaNeedham @JulienLamour. Here's my logic. Our goal is to translate the PFT parameter file estimate of Jmax (which is coming from a JVratio and Vcmax25 and a temperature response) to a cb6fmax.

We are calculating Je based on qabs (absorbed light) therefore we probably want to estimate cb6fmax from jmax based on an absorbed light (Qsat). Slightly simpler equation.

The assumption we are making is that the incident irradiance and absorptance used to measure and fit Vcmax & Jmax in the original studies (probably averaged and tuned) that were used to provide the data for the PFT specific parameterization is the same Qsat we use to estimate cb6fmax.

We just don't know what irradiance was used to measure the Aci curves used to derive Jmax but we are making the assumption that the physiologist probably used a somewhat standard ~1800 umol mol-1 of incident light and made the classic FvCB assumption that absorptance = 0.85 when estimating Jmax i.e. Jmax = J1800

This would give us a Qsat of 1530 umol mol-1. This is the new value of Qsat we will use (bumped up from the lower estimate to be more realistic). We're also assuming eta =1 and therefore 1^-1 =1 and we can simply the conversion to

cb6fmax = (Qsat * Jmax) / Qsat - (Jmax/phi)

Where Jmax is the Jmax from the PFT parameter file and where we are assuming phi for JB is equivalent to the FvCB phi. This simplification also aligns with the treatment of eta in our simulations i.e. we are using the Rectangular hyperbola type formulation without eta.

Does this make sense to you both?
A

@JulienLamour
Copy link

Hi @alistairrogers and @JessicaNeedham.

Ok for Qsat, I agree with Alistair's logic.

However, the conversion equation should be:

cb6fmax = eta * Qsat * Jsat / (Qsat - Jsat/Phi)

where Jsat = J_FvCB(Qsat) which is different from Jmax.

So there are two changes: Jmax should be Jsat, and the position of eta. Of course, considering eta = 1 then

cb6fmax = Qsat * Jsat / (Qsat - Jsat/Phi).

@alistairrogers
Copy link

@JulienLamour My understanding is that we need to take Jmax from the existing PFT parameter file (i.e. Jmax derived from a prescribed Vcmax25) and convert that Jmax from use in the FvCB formulation to the Cytb6f parameter for use in the JB formulation.

Because there is no Jsat in the existing FvCB code we are assuming that Jsat = J_FvCB(Qsat) for an incident irradiance (Qsat) of 1800. Therefore J_FvCB(Qsat) would be equal to Jmax estimated using FvCB with an incident irradiance of 1800. Does that make sense? Let's talk through this tomorrow.

@alistairrogers
Copy link

@JessicaNeedham Julien and I spoke this morning and all is clear. I was misunderstanding the situation. More to follow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Under Review
Development

Successfully merging this pull request may close these issues.

5 participants