Conversation
mbechtold
changed the title
mbechtold
changed the title
src/global.f90
Outdated
| PrevECdSComp | ||
|
|
||
| ! esnures consistency for adjusted compartment sizes between Pascal and Fortran | ||
| CropZx_eff = CropZx + ac_zero_threshold |
There was a problem hiding this comment.
(1) not quite understood why a small value needs to be consistently added in one direction (it is also not really serving as a zero threshold in the same sense as used earlier in the code) (2) typo "ensures"
There was a problem hiding this comment.
I had to do a similar trick in LDT to get the right number of compartments. I think it has basically the same result but instead of having a new CropZx_eff, I adjusted one if statement.
See L185 https://github.com/KUL-RSDA/LISF/blob/working/ac72_GMD/ldt/params/AquaCrop/define_AC_compartments.F90
I am not sure why this happens (small rounding errors, differences in compilation, optimization?), but this line was indeed dangerous
if (TotDepthC < CropZx) then
and is solved by adding a small value
There was a problem hiding this comment.
Yes, we had the problem earlier. It's because the user defines round values for the max root zone depth like 1.5. Such round values are directly at the edge when the compartment thickness calculation decides to add or remove a 0.05 increment to the total depth of the simulated soil depth. We struggled with that difference in how the Delphi and Pascal code eventually discretizes. Adding a very small value solved the problem. The epsilon was not enough, so I used the ac_zero_threshold which worked.
@gdelannoy It is correct that the meaning of ac_zero_threshold is a bit different. I could also hardcode the small number instead of using this parameter. Should I?
| tDaysZmin = real(L0, kind=dp) | ||
| tGDDZmin = real(GGDL0, kind=dp) | ||
|
|
||
| if ((GetSimulParam_RootPercentZmin() < 100) .and. (ZMin < ZMax)) then |
There was a problem hiding this comment.
100._dp? Not sure anymore if that was needed for >, < operations.
There was a problem hiding this comment.
It's best if we put dp I think (and also consistent with the rest of the code).
There was a problem hiding this comment.
Would say so too. No harm.
lbusschaert
left a comment
lbusschaert
left a comment
There was a problem hiding this comment.
Looks good! Only minor comments
src/global.f90
Outdated
| PrevECdSComp | ||
|
|
||
| ! esnures consistency for adjusted compartment sizes between Pascal and Fortran | ||
| CropZx_eff = CropZx + ac_zero_threshold |
There was a problem hiding this comment.
I had to do a similar trick in LDT to get the right number of compartments. I think it has basically the same result but instead of having a new CropZx_eff, I adjusted one if statement.
See L185 https://github.com/KUL-RSDA/LISF/blob/working/ac72_GMD/ldt/params/AquaCrop/define_AC_compartments.F90
I am not sure why this happens (small rounding errors, differences in compilation, optimization?), but this line was indeed dangerous
if (TotDepthC < CropZx) then
and is solved by adding a small value
| tDaysZmin = real(L0, kind=dp) | ||
| tGDDZmin = real(GGDL0, kind=dp) | ||
|
|
||
| if ((GetSimulParam_RootPercentZmin() < 100) .and. (ZMin < ZMax)) then |
There was a problem hiding this comment.
It's best if we put dp I think (and also consistent with the rest of the code).
| ! Adjust time after planting (tCalDay OR tCalGDD) to simulate root zone expansion | ||
| Zini = GetCrop_RootMin() * (real(GetSimulParam_RootPercentZmin(), kind=dp)/100._dp) | ||
|
|
||
| if ((GetCrop_RootMin() < GetCrop_RootMax()) .and. (ZiPrev > Zini)) then |
There was a problem hiding this comment.
Just wondering, is it even possible to have RootMin>RootMax? Shouldn't the code raise an error if it's defined like that in the crop parameters? Now if it does not enter the if statement, nothing happens
There was a problem hiding this comment.
It's probably there to avoid that this part is executed when RootMin == RootMax which then causes errors. True that RootMin should never be higher than RootMax, however, I don't think the current AquaCrop standalone catches non-meaningful parameter combinations elsewhere in the code. The GUI maybe.
src/run.f90
Outdated
| call CalculateRootingDepth(tDaysZmin,tGDDZmin,& | ||
| GetZiPrev(),GetGDDayi(),RootingDepth_temp) | ||
| call SetRootingDepth(RootingDepth_temp) | ||
| !call SetRootingDepth(AdjustedRootingDepth(GetPlotVarCrop_ActVal(), & |
There was a problem hiding this comment.
Leave commented? Or remove to clean up?
3 participants
This PR implements a mechanism for a more realistic expansion of the root zone after rewatering of the soil profile.
All V73 pull requests have been merged into a single version of the Fortran-based AquaCrop code. The outputs were verified against the Delphi-based standalone version that is the current engine of the AquaCrop GUI, with no differences observed in outputs.