CIMIS irrigation workflow, v1#3821
Conversation
mdietze
left a comment
There was a problem hiding this comment.
overall looks to be on the right track to knock of #3762. Before moving on the #3763 and #3764 I think the main thing that remains is to scale up the calculations to the full set of CA crop parcels using the HLS phenology data extracted by @sarahkanee. That said, if we can sneak any bits of #3763 into here I'd prioritize better defaults for minWHC
|
I made the mistake of initially developing this completely offline of PEcAn and, in doing so, completely missed #3765, which implements a sizeable chunk of this work (and does so with better code). Won't be that mistake again! So we should definitely hold on merging this until I refactor my work to use that code and eliminate my redundant implementations. That said, the refactor is a good opportunity to bang out the two companion issues mentioned above, at least in first pass form, since they seem like only minor straightforward tweaks to the water balance logic. Can you point me to where the HLS phenology data are? If so, I can integrate that at the same time as the other changes. |
|
I refactored this to use the BISM coefficients from #3765 . A nice side-effect is that the vignette now runs much faster, and the logic is simpler. I also vectorized the CHIRPS download code so we only read one file per year (rather than re-reading the same year's file separately for every day, like we were doing before) and updated some of the water balance docs. I still need to clean up the deprecated CIMIS data (I'll do that as a rebase to avoid inflating the repo size). I also need to make some of the changes to the water balance parameterization as suggested by @mdietze. Finally, and most importantly, I still need to integrate the HLS-based phenology information. |
62b12a5 to
e40ee6b
Compare
bd86959 to
2c8858f
Compare
|
OK, I think I'm ready to call this PR done. It seems like a lot, but I'm hoping the changes are relatively self-contained and straightforward to review. My suggestion for reviewing this is to start with the three new scripts in The demonstration scripts are as follows:
The most important science thing to check here is the Note also that I was unaware of #3643 when I wrote the SSURGO mukey functionality, so I inadvertently recreated a (possibly worse) version of EDIT: I tried out |
mdietze
left a comment
There was a problem hiding this comment.
@ashiklom there's a LOT here so I had to skip parts. A few quick questions before we merge
- Has this workflow been run to produce outputs (e.g. on the BU server) and if so is that just for the anchor sites or statewide? Please share file paths on Slack as applicable.
- If this has only been run for anchor sites, is there anything preventing a scaling up to statewide or any improvements you'd want to make before doing so?
- In writing events, are you still aggregating irrigation needs up to a weekly timescale or does the switch to irrigating to FC alleviate this need?
|
|
@ashiklom OK to merge this when checks pass? |
don't add extra steps.
|
OK, now this is ready for merge (or at least review). This now accurately reflects the actual statewide irrigation workflow we are using for the current batch of event files for the CCMMF MVP. A few major changes since last time:
Note that the CI The 3 packages I added are (1) |
So that `req_perform_parallel` is included in the package.
3d44a06
See #3762. This is a draft with significant stylistic and performance issues. See the
qmdfile for an end-to-end overview of the functionality.I definitely want to refine this, but I wanted to open it as a progress marker and to make confirm I'm on the right track.