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.

Sign up for GitHub

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

Jump to bottom

bucket model, AET values confusing when S=0 #188

Closed
dylanbeaudette opened this issue May 2, 2021 · 6 comments · Fixed by #189
Closed

bucket model, AET values confusing when S=0 #188

dylanbeaudette opened this issue May 2, 2021 · 6 comments · Fixed by #189

Comments

@dylanbeaudette
Copy link

dylanbeaudette commented May 2, 2021 •
edited
Loading

Issue summary:

ET (actual ET) values are larger than P[t] when S[t] = 0.

Expected Behavior

When S_prev = 0 AND P[t] << E[t], ET[t] should be limited to S[t].

Current Behavior

ET[t] is larger that P[t] when S_prev = 0, this doesn't make sense. See below.

Possible Solution

Constraints: (Etrans + Ebare) cannot be > S[t].

# line 46 of https://github.com/josephguillaume/hydromad/blob/master/R/bucket.R
# 2021-05-03
# DEB: (ET_trans + ETbare) cannot exceed S[t]
 ET[t] <- Eintc + min(S[t], (Etrans + Ebare))

Steps to Reproduce (for bugs)

d <- structure(list(P = c(65, 59, 57, 28, 13, 3, 0, 1, 4, 20, 33, 
53), E = c(14, 22, 38, 54, 92, 125, 154, 140, 106, 66, 29, 14
)), class = "data.frame", row.names = c(NA, -12L))

m <- hydromad::hydromad(d, sma = "bucket", routing = NULL)
m <- update(m, Sb = 47, fc = 1, S_0 = 1, a.ss = 0.01, M = 0, etmult = 1, a.ei = 0)
res <- predict(m, return_state = TRUE)

# combine original PPT,PET with results
res <- data.frame(d, res)

# cleanup names
names(res) <- c('PPT', 'PET', 'U', 'S', 'ET')
| PPT| PET|      U|      S|     ET|
|---:|---:|------:|------:|------:|
|  65|  14| 51.000| 47.000| 14.000|
|  59|  22| 37.000| 47.000| 22.000|
|  57|  38| 19.000| 47.000| 38.000|
|  28|  54|  0.000| 21.000| 54.000|
|  13|  92|  0.000|  0.000| 66.553|
|   3| 125|  0.000|  0.000|  7.979|
|   0| 154|  0.000|  0.000|  0.000|
|   1| 140|  0.000|  0.000|  2.979|  <---
|   4| 106|  0.000|  0.000|  9.021|  <---
|  20|  66|  0.000|  0.000| 28.085|
|  33|  29|  0.000| 12.638| 20.362|
|  53|  14|  4.638| 47.000| 14.000|

Context

Monthly water balance.

Your Environment

R version 4.0.2 (2020-06-22)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Catalina 10.15.7

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/4.0/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] hydromad_0.9-26     reshape_0.8.8       polynom_1.4-0       latticeExtra_0.6-29 lattice_0.20-41    
[6] zoo_1.8-9           soilDB_2.5.9        aqp_1.30            sharpshootR_1.8.1
@dylanbeaudette dylanbeaudette mentioned this issue May 2, 2021
Closed
4 tasks
@buzacott
Copy link

Thanks for reporting the issue and apologies about the delay. I've amended the code based on your suggested fix and everything looks good.

> res
   PPT PET         U       S      ET
1   65  14 51.000000 47.0000 14.0000
2   59  22 37.000000 47.0000 22.0000
3   57  38 19.000000 47.0000 38.0000
4   28  54  0.000000 28.0000 47.0000
5   13  92  0.000000  0.0000 41.0000
6    3 125  0.000000  0.0000  3.0000
7    0 154  0.000000  0.0000  0.0000
8    1 140  0.000000  0.0000  1.0000
9    4 106  0.000000  0.0000  4.0000
10  20  66  0.000000  0.0000 20.0000
11  33  29  0.000000 12.6383 20.3617
12  53  14  4.638298 47.0000 14.0000

I'll open a PR which will hopefully be merged soon

@buzacott buzacott mentioned this issue May 15, 2021
Merged
3 tasks
@dylanbeaudette
Copy link
Author

This is great, thank you! I've been using hydromad for a long time and really appreciate the package. The speed of the C implementation is a nice bonus.

@josephguillaume
Copy link
Collaborator

Thanks for reporting the issue, and for such a perfect reproducible example!
We're obviously still working through process issues to try to get a timely response... Thanks @buzacott for putting together the PR, and apologies to both of you that it took me so long to get to.

@dylanbeaudette
Copy link
Author

No worries and thank you both for working on this. I'd really like to build more of our (USDA/NRCS) workflows on current / planned functionality in hydromad. Maybe we can schedule some time to talk about the more complex models described in the main references.

@josephguillaume
Copy link
Collaborator

@dylanbeaudette - very happy to talk about this. Feel free to email me at joseph.guillaume@anu.edu.au

@dylanbeaudette
Copy link
Author

Will do, thanks again.

I went back to a project from 2019 that used bucket.sim to compute a simple water balance for soils in CA, assuming a managed water storage capacity of 4 inches. This is a concept from the 60's that is still widely used for the CA Land Capability Class. In short, the latest version of hydromad does a more realistic job estimating ETa in the driest parts of the state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ET in bucket model exceeding available water buzacott/hydromad
3 participants
@dylanbeaudette @josephguillaume @buzacott