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

Improved/fixed sorghum co2 response #5479

Merged
merged 5 commits into from
Aug 20, 2020

Conversation

hol430
Copy link
Contributor

@hol430 hol430 commented Aug 3, 2020

Working on #572

@jbrider - with this change we get some co2 response but not much, and we probably will need to reparameterize the two functions I copied from maize. We may not necessarily want to merge this change, I'm mainly interested to see what effect it has on the stats (if any).

I tried to test the sorghum model model from old apsim with variable co2 levels, but it fails because there's no CO2xTE response defined in Sorghum.xml.

- Changed Sorghum.Leaf.Photosynthesis.FCO2 to a RUECO2Function with its
  pathway set to "C4". This will behave the same as the
  rue_co2_modifier defined in the sorghum model from old apsim. I also
  removed a memo from here which said

  > Replaced the RUE2CO2 function as C4 Photosynthetic pathway is not
  > represented in old sorghum. Look to reimplement it once the stats
  > are working

- I also had to copy the StomatalConductanceCO2Modifier and CO2Internal
  functions from maize - the stomatal modifier was previously a
  constant of 1. This may have to be reparameterized to work properly
  with sorghum, I don't really know.
@Keith-Pembleton
Copy link

have a look at https://doi.org/10.1017/S0021859615001185 for some sorghum parameters. They were developed for forage sorghum in classic but they will at least give a starting point to work from.

@jbrider
Copy link
Contributor

jbrider commented Aug 3, 2020

thanks @Keith-Pembleton. We can use this to help test NextGen as well.
There should have been defaults in there that must have been removed at some stage - will need to look back in version control to find them.

@jbrider
Copy link
Contributor

jbrider commented Aug 3, 2020

@hol430 You will probably need to bring the CO2 code across from sorghum classic in the first instance as it looks like it works differently to nextgen maize. I think the CO2 response is integrated with MicroClimate which may not work with sorghum yet. I have forwarded a couple of papers that Graeme sent me - which should be a reflection of what is implemented in the sorghum and maize classic code.

The shifting influence of drought and heat stress for crops in northeast Australia - Lobell, David & Hammer, G. & Chenu, Karine & Zheng, Bangyou & McLean, Greg & Chapman, Scott. (2015)

Designing crops for adaptation to the drought and high temperature risks anticipated in future climates. Hammer, G. & McLean, Greg & Oosterom, Erik & Chapman, S. & Zheng, Bangyou & Wu, Alex & Doherty, Al & Jordan, D.. (2020).

@hol430
Copy link
Contributor Author

hol430 commented Aug 3, 2020

Thanks Jason. From what I can see, the only place that co2 has an effect in old sorghum model is in the calculation of RUE

// Plant.cpp:274
float rueToday = (float)(rue * rue_co2_modifier());
...
// Plant.cpp:539
double Plant::rue_co2_modifier(void)                 //!CO2 level (ppm)
   {
   //  Purpose : Calculation of the CO2 modification on rue
   const double scale = 1.0 / 350.0 * 0.05;
   return (scale * this->co2 + 0.95); //Mark Howden, personal communication
   }

This is already implemented in next gen as the RUECO2Function:

// RUECO2Function.cs:67
else if (PhotosyntheticPathway == "C4")
{
    return 0.000143 * MetData.CO2 + 0.95; //Mark Howden, personal communication
}

That's one of the changes I've made in this pull request - I've changed the FCO2 function (previously a constant of 1) in next gen sorghum to use the RUECO2Function with the pathway set to "C4". The other changes I made were because GSMax (max daily stomatal conductance) was always returning 0, due to GSMax350 and FRGR always being 0.

  • I set GSMax350 to constant of 0.009, which is what maize does
  • I connected the FRGR property to the child function of the same name (previously this property was just returning 0).
  • I changed StomatalConductanceMod (previously constant of 1) to use the imlpementation in maize (FCO2/([IWeather].CO2 - [Leaf].CO2internal)/(350 - [Leaf].CO2internal)). This may not have been the right thing to do, as the old sorghum doesn't seem to have this modifier as far as I can tell

@Keith-Pembleton
Copy link

@hol430 from memory you are right sorghum in classic was structured different to the other models. We used the forage/sweet sorghum model in the past. I think adding what maize does to sorghum gives us a place to start from. @JJguri, I and others will be working on it over the next few months.

Numbers taken straight from maize in old apsim
@hol430
Copy link
Contributor Author

hol430 commented Aug 7, 2020

It appears that sorghum's co2 response curve in old apsim was removed in 2012, but even before the parameters were removed from the xml file, there was no response - it was just a flat line. When the parameters were removed from the xml file, this was changed to be an optional parameter, so the model will work with them or without them. Maize in classic is only different in the sense that these parameters are not optional, and it provides some default values.

In new apsim, we had a co2 modifier on RUE but not on TE. I've now added a CO2xTE response, but the question is how do we want to parameterize it. For now I've just copied the response used in the maize model in apsim classic but I'm happy to change this to whatever you guys reckon is best, just let me know. There are quite a few parameterizations in the paper linked by Keith which we could try. If anyone wants to have a play, the CO2TE experiment in the sensibility folder should give a decent graph

@hol430
Copy link
Contributor Author

hol430 commented Aug 18, 2020

Ok, after discussion with @jbrider I have changed the sorghum CO2xTE response to match what old apsim did - ie there is no response, it's a flat line. For those of you wanting to change/experiment with this, it lives at Sorghum.Leaf.PotentialBiomassTEFunction.CO2Modifier.

@hol430
Copy link
Contributor Author

hol430 commented Aug 18, 2020

@hol353, @jbrider I think we can accept the stats on this one - nse for Leaf.Dead.Wt has changed by ~0.01.

@jbrider
Copy link
Contributor

jbrider commented Aug 18, 2020

@Keith-Pembleton @JJguri Let us know if you have trouble with setting your own values for it.

@JJguri
Copy link
Contributor

JJguri commented Aug 18, 2020

@Keith-Pembleton @JJguri Let us know if you have trouble with setting your own values for it.

Thanks @jbrider, we will be working on it during the next weeks.

@hol430
Copy link
Contributor Author

hol430 commented Aug 19, 2020

@hol353 should be safe to accept stats here (as per above comment, NSE for one sorghum variable has changed by ~0.01).

@hol353
Copy link
Contributor

hol353 commented Aug 20, 2020

retest this please jenkins.

Want to rerun this because I merged another PR this morning.

@hol353 hol353 merged commit 4d4f9c4 into APSIMInitiative:master Aug 20, 2020
@jbrider jbrider mentioned this pull request Aug 27, 2020
@hol430 hol430 deleted the bug/SorghumCO2 branch September 1, 2020 23:38
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.

5 participants