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

Documentation for soil moisture deficit calculation is incorrect. #872

Closed
yanyanchengHydro opened this issue Dec 20, 2019 · 8 comments · Fixed by #2220 or #2230
Closed

Documentation for soil moisture deficit calculation is incorrect. #872

yanyanchengHydro opened this issue Dec 20, 2019 · 8 comments · Fixed by #2220 or #2230
Assignees
Labels
documentation additions or edits to user-facing documentation

Comments

@yanyanchengHydro
Copy link
Contributor

yanyanchengHydro commented Dec 20, 2019

Brief summary of bug

As indicated in page 247 of the CLM5 documentation, the soil moisture deficit to calculate irrigation should be equal to “soil moisture threshold - available moisture". But in IrrigationMod.F90, it uses “irrigation target soil moisture” rather than “soil moisture threshold” to minus the “available moisture”. Then this situation is equal to that the irrigation threshold fraction is always equal to 1.

General bug information

CTSM version you are using: ctsm1.0.dev080
Does this bug cause significantly incorrect results in the model's science? [No]

Details of bug

Soil moisture deficit calculation in CLM5 documentation
Screenshot 2019-12-20 11 27 44

Bug in the source code IrrigationMod.F90

Screenshot 2019-12-20 11 11 58

@ekluzek @swensosc @dlawrenncar @huangmy

@billsacks billsacks added the documentation additions or edits to user-facing documentation label Dec 21, 2019
@billsacks
Copy link
Member

Thank you for bringing this to our attention. I have been looking at this code, and I agree that the code differs from the documentation. However, I think that the code is right and the documentation is wrong here.

I haven't taken the time to get my head 100% back into this, but I think that h2osoi_liq_target is truly supposed to give the target soil moisture value that we're shooting for whenever irrigation happens; then the deficit gives the difference between this target value and the current soil moisture. h2osoi_liq_at_threshold, on the other hand, gives a threshold at which we decide to do any irrigation at all. The way this is written allows for the possibility that you may not want to irrigate every time there becomes even a tiny soil moisture deficit. Instead, you may want to wait until the deficit is larger before initiating irrigation; at that point, you don't want to just irrigate up to the "threshold" but instead up to the higher "target". The target should always be greater than or equal to the threshold.

Regarding this:

Then this situation is equal to that the irrigation threshold fraction is always equal to 1.

Note that the threshold fraction is still relevant for the conditional just above your proposed change; it just isn't invoked (nor should it be) in actually calculating the deficit.

Unless someone thinks I'm misunderstanding something here, I believe the course of action is that we should fix the documentation to be consistent with the (correct) code.

(Also including @danicalombardozzi in this discussion.)

@dlawrenncar
Copy link
Contributor

@swensosc may also have thoughts about this.

@swensosc
Copy link
Contributor

swensosc commented Dec 23, 2019 via email

@danicalombardozzi
Copy link
Contributor

@billsacks and @swensosc: @yanyanchengHydro also pointed out that this code results in the irrigation threshold fraction always being equal to 1. Is that the correct behavior? Or should it be able to vary?

@billsacks
Copy link
Member

@billsacks and @swensosc: @yanyanchengHydro also pointed out that this code results in the irrigation threshold fraction always being equal to 1. Is that the correct behavior? Or should it be able to vary?

I'm not sure exactly what is meant by this. As written, the deficit is based on h2osoi_liq_target_tot, which is equal to h2osoi_liq_at_threshold if irrig_threshold_fraction is 1. So maybe that's what @yanyanchengHydro is referring to? But I still believe that the code is written as intended.

I'll wait to move ahead with this until hearing back from @yanyanchengHydro

@yanyanchengHydro
Copy link
Contributor Author

@billsacks @swensosc @danicalombardozzi @dlawrenncar Thank you Bill for the clarification. Yes, I mean the deficit is based on h2osoi_liq_target_tot, which is equal to h2osoi_liq_at_threshold if irrig_threshold_fraction is 1.

I did not correctly understand the purpose of irrig_threshold_fraction at the very beginning. Since the motivation for irrig_threshold_fraction is to avoid irrigating for small deficits, I agree with Bill that the code is correct rather than the documentation.

@billsacks billsacks self-assigned this Jan 10, 2020
@ekluzek ekluzek changed the title Bug for soil moisture deficit calculation in IrrigationMod.F90 Documentation for soil moisture deficit calculation is incorrect. Feb 5, 2021
@samsrabin
Copy link
Collaborator

@billsacks We talked about this in the CTSM SE meeting yesterday, and @olyson mentioned he'd be willing to fix the docs if indeed the code is correct. If that's the case, would you be okay with him taking this on as the new assignee?

@billsacks
Copy link
Member

@billsacks We talked about this in the CTSM SE meeting yesterday, and @olyson mentioned he'd be willing to fix the docs if indeed the code is correct. If that's the case, would you be okay with him taking this on as the new assignee?

Sounds great - thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation additions or edits to user-facing documentation
Projects
None yet
7 participants