Closed rcichota closed 9 months ago
rcichota
commented
3 years ago I have found another error in the model. The initialisation procedure was doubling the initial DM amount in roots. This was probably due to the somewhat confusing way that the roots are initialised, and related to the reset method. I will clean that up, hopefully making a bit easier to understand/maintain the code.
hut104
commented
3 years ago From: Rogerio Cichota @.> Sent: Monday, 6 September 2021 6:44 AM To: APSIMInitiative/ApsimX @.> Cc: Subscribed @.***> Subject: [APSIMInitiative/ApsimX] AgPasture update and fixes (#6767)
I (and others) have noticed something strange with AgPasture results. Its simulations are producing less than expected and favouring clover over ryegrass too much. After some checks I found that the issue is with how the model is calculating N available in the soil, this limits way too much the ability of plants to take N up. This probably emerged when Dean cleaned up the code and defined the uptake routine to be that of DefaultApsim. The original code in NextGen was quite messy as I misunderstood the different factors affecting N availability when converting from Classic. The routine/method (which mimics that of PMF) has this weird multiplication of no3 times no3ppm, which was removed. However, the code was correct, if messy. That multiplication does exist, but it would make a lot more sense if the code was written a bit different. The approach computes N available as being the amount of NO3 or NH4 times a moisture factor (the drier the soil, the smaller the N availability) and what should have been identified as a 'concentrationFactor' (the lower the concentration, the smaller the N availability). Using NO3 as example, this factor is equal to Min(1.0, kno3×no3ppm). The fact that this factor was not explicitly defined was in the root of the errors in adapting this approach to AgPasture. I am now fixing that. I wonder if it would be better to define this function differently, changing the meaning of the kno3 parameter. I think it would be more intuitive to use Min(1.0, no3ppm/no3ppmCritical); where no3ppmCritical is the concentration in the soil below which N availability becomes limited... While investigating the issue above I noticed a couple of other errors in the code as well as some typos and unclear tags and comments. I will be fixing and updating these here too (some are inconsequent the other is related to the on above). The first error I noticed was that the root length density has its unit wrong (by a a factor of 1000), I believe this was my mistake, converting m/m3 to mm/mm3, instead of m/m2 to mm/mm2. This variable and related code are not used anywhere yet (except output), so should have no consequence... The other error was on the implementation of the code to partition light between species (or re-partition after MicroClimate). This stems from the fact that AgPasture computes photosynthesis using a photosynthetic rate (or carbon assimilation rate) which instead of RUE as is the case of PMF. In Ecomod and in Classic the partition of light is done internally by AgPasture, whereas in NextGen each species is self-contained and the routine is more or less unaware of the other species. In its original implementation the routine uses actual radiation (called radiation on top of canopy), not intercepted; then it computes the absorption for whole sward and only in the end it partitions the photosynthesis between species. This cannot be implemented in NextGen, so I used an approach where the ratio between MicroClimate's intercepted radiation and actual radiation would give this conversion from top of canopy to whole canopy. This is not quite right and changes photosynthesis estimates in a 'chaotic' manner, over- or under-estimating depending on the proportion of different species and even dead leaves. Upon investigation I noticed that is possible to mimic the original approach by re-interpreting the original absorption function, adjusting it using the ratio of intercepted radiation from MicroClimate (each species divided by total interception). This may increase photosysnthesis too much with current parameters and we'll have to have a look at it...
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/APSIMInitiative/ApsimX/issues/6767, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC2UVWQMWHDWCFYAXTSZR5DUAPJD3ANCNFSM5DPFQOCA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
rcichota
commented
3 years ago Hi @hut104 Just to clarify, I am not intending or proposing we change the N uptake. Indeed, we wanted to make it use the same approach as PMF, we just miss-interpreted causing the error I am fixing it now. I understand what you say about the meaning of KNO3, I just think it is not very intuitive to think about the process that way. My point was that using either Min(1.0, kno3×no3ppm) or Min(1.0, no3ppm/no3ppmCritical) is exactly the same. It means that kno3 = 1/no3ppmCritical. For me, using no3ppmCritical is more intuitive, but maybe its just me. For people that got their mind around the use of kno3 the alternative may be just as difficult to grasp, but I'd hope not and maybe would be easier for newbies...??
rcichota
commented
3 years ago Regarding the light interception. I am tidying up/fixing the current routine to firstly mimic what was the intended approach (originally in Ecomod/Dairymod...). I fully agree that would be ideal to use canopy layering when accounting for aboveground competition. I will see where I get to, but I see a few potential stumbling blocks. Firstly the original approach did not consider variations in plant height, so it may need tweaks, or on its parameters, to maintain performance. Moreover, giving the patchy nature of pastoral communities it is not always the case that taller plants would shade the smaller, there certainly is some effect, but its extent may not be straight forward as it would be the case with a silvo-pastoral setting... To make more clear the issue at hand, AgPasture computes photosynthesis using the Thornley and Johnson method whereby photosynthesis is first computed at leaf level (this uses a non‐rectangular hyperbola which has an asymptote equal to Pmax, the potential or maximum CO2 assimilation rate). The conversion from leaf to canopy photosynthesis is done by assuming that Pmax varies across the canopy depth in the same way that light interception, this yields a relatively simple function but needs that leaf photosynthesis be calculated using radiation at the top of canopy (not the total intercepted). To continue using the same approach and use MicroClimate I think we'll need either modify MicroClimate to output this radiation on top of canopy, or to find a way to back calculate an equivalent value within AgPasture from the outputs we have now...
rcichota
commented
3 years ago Just pointing this issue to a few people, for awareness and welcoming any comments, @sno036 @MarkLieffering @mpandreucci @HamishBrownPFR
MarkLieffering
commented
3 years ago @rcichota - thank you for your efforts!
sno036
commented
3 years ago @rcichota - thanks for all of this. Would be good to talk over a few questions that I have. Reporting R0 (radiation intensity at the top of the canopy) should be easy and I think was even there in the original Fortran version. Will catch you for a discussion at some point.
hut104
commented
3 years ago Should be possible to calculate from existing data from microclimate. Current users include shading from plants in other zones, such as in agroforestry.
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Rogerio Cichota @.> Sent: Tuesday, September 7, 2021 6:27:14 AM To: APSIMInitiative/ApsimX @.> Cc: Huth, Neil (A&F, Toowoomba) @.>; Mention @.> Subject: Re: [APSIMInitiative/ApsimX] AgPasture update and fixes (#6767)
Regarding the light interception. I am tidying up/fixing the current routine to firstly mimic what was the intended approach (originally in Ecomod/Dairymod...). I fully agree that would be ideal to use canopy layering when accounting for aboveground competition. I will see where I get to, but I see a few potential stumbling blocks. Firstly the original approach did not consider variations in plant height, so it may need tweaks, or on its parameters, to maintain performance. Moreover, giving the patchy nature of pastoral communities it is not always the case that taller plants would shade the smaller, there certainly is some effect, but its extent may not be straight forward as it would be the case with a silvo-pastoral setting... To make more clear the issue at hand, AgPasture computes photosynthesis using the Thornley and Johnson method whereby photosynthesis is first computed at leaf level (this uses a non\rectangular hyperbola which has an asymptote equal to Pmax, the potential or maximum CO2 assimilation rate). The conversion from leaf to canopy photosynthesis is done by assuming that Pmax varies across the canopy depth in the same way that light interception, this yields a relatively simple function but needs that leaf photosynthesis be calculated using radiation at the top of canopy (not the total intercepted). To continue using the same approach and use MicroClimate I think we'll need either modify MicroClimate to output this radiation on top of canopy, or to find a way to back calculate an equivalent value within AgPasture from the outputs we have now...
― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/APSIMInitiative/ApsimX/issues/6767#issuecomment-913842422, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC2UVWSYUTAPV3AZHBHTBKDUAUP2FANCNFSM5DPFQOCA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
hut104
commented
3 years ago I can see what you are saying, and I think this has been discussed on other threads. I think previously the discussion ended up thinking about replacing this approach totally, with known approaches such as the Dewilligen approaches for NO3 and NH4 uptake. Some of these are already partially added into the agroforestry work, but not well tested.
From: Rogerio Cichota @.> Sent: Tuesday, 7 September 2021 6:06 AM To: APSIMInitiative/ApsimX @.> Cc: Huth, Neil (A&F, Toowoomba) @.>; Mention @.> Subject: Re: [APSIMInitiative/ApsimX] AgPasture update and fixes (#6767)
Hi @hut104https://github.com/hut104 Just to clarify, I am not intending or proposing we change the N uptake. Indeed, we wanted to make it use the same approach as PMF, we just miss-interpreted causing the error I am fixing it now. I understand what you say about the meaning of KNO3, I just think it is not very intuitive to think about the process that way. My point was that using either Min(1.0, kno3×no3ppm) or Min(1.0, no3ppm/no3ppmCritical) is exactly the same. It means that kno3 = 1/no3ppmCritical. For me, using no3ppmCritical is more intuitive, but maybe its just me. For people that got their mind around the use of kno3 the alternative may be just as difficult to grasp, but I'd hope not and maybe would be easier for newbies...??
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/APSIMInitiative/ApsimX/issues/6767#issuecomment-913834477, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC2UVWQ3VGRVY6T7QTCHLVLUAUNLPANCNFSM5DPFQOCA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
Keith-Pembleton
commented
3 years ago not related to the method here, but should consideration be given to the convention? KNO3 to me is potassium nitrate (a fertiliser). Could this become a problem if there is at some point in the future the implementation a potassium fertiliser/nutrition component?
par456
commented
9 months ago From what I can read in this issue and the PRs, this issue was solved in the merge above, so I will close the issue. It wasn't auto-closed because it had "Working on" instead of resolves.
I (and others) have noticed something strange with AgPasture results. Its simulations are producing less than expected and favouring clover over ryegrass too much. After some checks I found that the issue is with how the model is calculating N available in the soil, this limits way too much the ability of plants to take N up. This probably emerged when Dean cleaned up the code and defined the uptake routine to be that of DefaultApsim. The original code in NextGen was quite messy as I misunderstood the different factors affecting N availability when converting from Classic. The routine/method (which mimics that of PMF) has this weird multiplication of no3 times no3ppm, which was removed. However, the code was correct, if messy. That multiplication does exist, but it would make a lot more sense if the code was written a bit different. The approach computes N available as being the amount of NO3 or NH4 times a moisture factor (the drier the soil, the smaller the N availability) and what should have been identified as a 'concentrationFactor' (the lower the concentration, the smaller the N availability). Using NO3 as example, this factor is equal to Min(1.0, kno3×no3ppm). The fact that this factor was not explicitly defined was in the root of the errors in adapting this approach to AgPasture. I am now fixing that. I wonder if it would be better to define this function differently, changing the meaning of the kno3 parameter. I think it would be more intuitive to use Min(1.0, no3ppm/no3ppmCritical); where no3ppmCritical is the concentration in the soil below which N availability becomes limited... While investigating the issue above I noticed a couple of other errors in the code as well as some typos and unclear tags and comments. I will be fixing and updating these here too (some are inconsequent the other is related to the on above). The first error I noticed was that the root length density has its unit wrong (by a a factor of 1000), I believe this was my mistake, converting m/m3 to mm/mm3, instead of m/m2 to mm/mm2. This variable and related code are not used anywhere yet (except output), so should have no consequence... The other error was on the implementation of the code to partition light between species (or re-partition after MicroClimate). This stems from the fact that AgPasture computes photosynthesis using a photosynthetic rate (or carbon assimilation rate) which instead of RUE as is the case of PMF. In Ecomod and in Classic the partition of light is done internally by AgPasture, whereas in NextGen each species is self-contained and the routine is more or less unaware of the other species. In its original implementation the routine uses actual radiation (called radiation on top of canopy), not intercepted; then it computes the absorption for whole sward and only in the end it partitions the photosynthesis between species. This cannot be implemented in NextGen, so I used an approach where the ratio between MicroClimate's intercepted radiation and actual radiation would give this conversion from top of canopy to whole canopy. This is not quite right and changes photosynthesis estimates in a 'chaotic' manner, over- or under-estimating depending on the proportion of different species and even dead leaves. Upon investigation I noticed that is possible to mimic the original approach by re-interpreting the original absorption function, adjusting it using the ratio of intercepted radiation from MicroClimate (each species divided by total interception). This may increase photosysnthesis too much with current parameters and we'll have to have a look at it...