USDA-ARS-ACSL / PhotoSynthesisModule

Stand alone C++ module to simulate Farquhar Ball-Berry model of photosynthesis and transpiration
The Unlicense
12 stars 1 forks source link

Iteration bugs in energy balance #2

Open rafaqz opened 6 years ago

rafaqz commented 6 years ago

Thanks for making this module available.

It seems that the while loop in EnergyBalance is not actually iterative, and will just repeat the same calculations until maxiter, as newTi or lastTi do not feed back into any calculations. I'm not sure what this means for energy balance estimates.

ARS-CSGCL-DT commented 6 years ago

Hi Rafael,

I’m really happy to see that someone is looking at this. There could be a bug in the version I took out of MAIZSIM.

The EnergyBalance method calculates a leaf temperature (TLeaf) based on the temperature and irradiance. The equation to calculate leaf temperature from radiance and transpiration is highly non-linear so we iterate to get a better answer.

  1. Save the last calculated value of leaf temperature in lastTi (first time through the loop it is set to the current Tleaf and newTi is set to -10.
  2. Calculate Tleaf and a residual
  3. Calculate a new value of Tleaf and call it newTi. The while loop will compare the new temperature (newTi) to the value from the previous iteration (lastTi) and end if the difference is small. The comparison and decision is made in the while statement (while ((abs(lastTi-newTi)>0.001) && (iter <maxiter)))

Overall

Save last calculation Calculate a new value of leaf temperature Check if the while loop should end by comparing the new and old temperatures End if they are not different, otherwise continue

The real work is done in the The Gas_Exchange method which does the iteration to solve leaf temperature (based on photosynthetic rate and the energy balance). It iterates until the leaf temperature stabilizes

Now that I think of it, this function should not be called energy balance since the balance is not calculated here.

But, we actually removed the looping in the EnergyBalance function in MAIZSIM because the initial temperatures were not all that different from the final ones and the Gas_Exchange module was the real calculator of leaf temperature.

Does this answer your question?

How are you using the module? Any other feedback? Please feel free to ask other questions.

Best wishes, Dennis


Dennis Timlin Soil Scientist USDA-ARS Adaptive Cropping Systems Laboratory Bldg 001, Rm 342 BARC-W 10300 Baltimore Ave Beltsville, MD 20705 301-504-6255 fax 301-504-5823 Dennis.Timlin@ars.usda.gov http://www.ars.usda.gov/nea/barc/acsl

From: Rafael Schouten [mailto:notifications@github.com] Sent: Wednesday, February 28, 2018 7:47 AM To: ARS-CSGCL-DT/PhotoSynthesisModule PhotoSynthesisModule@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: [ARS-CSGCL-DT/PhotoSynthesisModule] Iteration bugs in energy balance (#2)

Thanks for making this module available.

It seems that the while loop in EnergyBalance is not actually iterative, and will just repeat the same calculations until maxiter, as newTi or lastTi do not feed back into any calculations. I'm not sure what this means for energy balance estimates.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/ARS-CSGCL-DT/PhotoSynthesisModule/issues/2, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEcoJKo-GcqvmXMhquj7SLq7Be67fOdGks5tZUqsgaJpZM4SWmKp.

This electronic message contains information generated by the USDA solely for the intended recipients. Any unauthorized interception of this message or the use or disclosure of the information it contains may violate the law and subject the violator to civil or criminal penalties. If you believe you have received this message in error, please notify the sender and delete the email immediately.

rafaqz commented 6 years ago

Thanks for your response!

I realised later that the gas exchange function does the main work. But still having the where loop in energy balance is confusing, and uses precious cpu cycles! There are quite a few lines of code around the model that are not used like this, especially in the C4 function, and other parts of photosynthesis that can be pulled out of the inside loops. My latest version is a lot faster for the same output (i need to run it ~1,000,000x a second!)

Unfortunately I can't put in a pull request as I've converted the code to the julia language for easier interop with my plant growth models, but I will eventually share the julia package on github.

ARS-CSGCL-DT commented 6 years ago

Hi Rafael,

Sounds like you have made some useful changes, I wish I were a better programmer to make things more efficient. Also, the efficiency of the plant model is not that much of an issue now. We have a finite element soil model that pretty much dominates the cpu cycles. I’m trying to develop an adaptive grid that would help speed things up.

How are you using your plant models?

Best wishes, Dennis

From: Rafael Schouten [mailto:notifications@github.com] Sent: Friday, March 2, 2018 2:40 AM To: ARS-CSGCL-DT/PhotoSynthesisModule PhotoSynthesisModule@noreply.github.com Cc: Timlin, Dennis Dennis.Timlin@ARS.USDA.GOV; Comment comment@noreply.github.com Subject: Re: [ARS-CSGCL-DT/PhotoSynthesisModule] Iteration bugs in energy balance (#2)

Thanks for your response!

I realised later that the gas exchange function does the main work. But still having the where loop in energy balance is confusing, and uses precious cpu cycles! There are quite a few lines of code around the model that are not used like this, especially in the C4 function, and other parts of photosynthesis that can be pulled out of the inside loops. My latest version is a lot faster for the same output (i need to run it ~1,000,000x a second!)

Unfortunately I can't put in a pull request as I've converted the code to the julia language for easier interop with my plant growth models, but I will eventually share the julia package on github.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/ARS-CSGCL-DT/PhotoSynthesisModule/issues/2#issuecomment-369845995, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEcoJNuZsGTO0h_DsQXJrChq9mgLhUzAks5taPczgaJpZM4SWmKp.

This electronic message contains information generated by the USDA solely for the intended recipients. Any unauthorized interception of this message or the use or disclosure of the information it contains may violate the law and subject the violator to civil or criminal penalties. If you believe you have received this message in error, please notify the sender and delete the email immediately.

rafaqz commented 6 years ago

Sure, it only makes sense to optimise the bottlenecks. For me photosynthesis is the bottleneck!

I'm using your photosynthesis/energy budget model as part of a species-distribution modelling framework for plants. I'm currently playing with coupling this component to a Dynamic Energy Budget and/or Thornley Transport Resistance models of plant growth.

I'm generating local microclimates using the NichemapR package, and we will hopefully be able to predict some kind of spatial patterns of habitat suitability if we have a physiological plant model that behaves realistically enough in response to local climatic variables. I was writing this photosynthesis/energy budget part out myself from scratch before realising it could be a whole thesis worth of work by itself!! So it was great to find this repository.

I'll let you know when I publish the set packages I'm working on. It will include a package based on this one but hopefully faster and reorganised a little (and fully credited of course!!). It was very relieving to see that this module has such a liberal license so I can use it however I need to. I'm using Julia as it makes this kind of work a lot easier than C++, and its very easy to make packages that interact with each other cleanly even in inner loops, so I'm migrating a lot of plant physiology packages to Julia, hopefully that turns out to be a good decision!!