Error: no such column: ps_cumulative_mgd from running summarize_element.sh

[glenncampagna]

Example:

sudo -u www-data ./modules/om/sh/summarize_element.sh 328319 11
Error: no such column: ps_cumulative_mgd

The above example is from Carvin's which would call waterSupplyModelNode.R which. contains (lines 136 - 144):

if ("ps_cumulative_mgd" %in% cols) { #cumulative variables are needed for calculations including Water Availability and combine all upstream contributions
  ps_cumulative_mgd <- mean(as.numeric(dat$ps_cumulative_mgd) )
  if (is.na(ps_cumulative_mgd)) {
    ps_cumulative_mgd = ps_mgd
  }
} else {
  ps_cumulative_mgd = ps_mgd
  dat$ps_cumulative_mgd <- dat$ps_mgd
}

• In the case of Carvins, no ps_mgd is in the original timeseries data, so we cannot assume it already exists
• [x] Check if ps_mgd exists, create col of 0 if not

[rburghol]

@glenncampagna we should look for the variable "child_ps_mgd" before guestimating as 0.0 ~this looks like a good solution~ as this is one of those tributary style templates. I am a bit puzzled that this has not arisen before since Carvin's, and many, many other trib templates have used the child_ps_mgd (and child_wd_mgd) variables, and these summaries have long been part of our modeling suite, and have been run without incident. Can you check that no recent(ish) pull requests have over-written old code that properly handled the child_ps_mgd and wd variables?

[rburghol]

Hey @glenncampagna -- I wanted to share some search techniques with you since I asked "Can you check that no recent(ish) pull requests have over-written old code".

But first, you don't actually need to do any searching as I think that you have discovered a bug that is somewhat unique to Carvin's cove model segment, and triggered by enhancements that have been made to the baseline flow calcs. I think that the enhancements that we made are clearly necessary, and so, this becomes a bug in Carvin's, not a bug in our script. Carvin's lacks the variable in question, as you observed, but most other tribs do not -- Carvin's is a really old model that has never been wholly replaced, just "renovated" as it were... thus, the missing variable(s).

Anyhow, great work on the discovery, and here is what I wanted to share -- how to search the commit log in git for code fragments in the event that you did need to search sometime in the future. In this case, I decided to search for ps_mgd in the OM repo commit log to see if anything that I had committed in the last couple months somehow deleted some code.

I cd'ed into the git om directory and used: git log -S "ps_mgd" --source --all, which returned:

commit c2b5d4e245d865a5e76b16712b2174c3adfdc1b0 refs/heads/deb5
Author: glenncampagna <gcampagna11@vt.edu>
Date:   Mon Sep 18 21:03:58 2023 -0400

    remove CPL from and finalize function, FDC message for neg values

commit edfc2e0202019b5767a380d8df258cd52da02878 refs/heads/deb5
Author: Burgholzer <Robert.Burgholzer@deq.virginia.gov>
Date:   Mon Jul 31 14:33:19 2023 -0400

    Insure hydro imp reports 0 storage remaining if impoundment is inactive

commit 2ea9af932a3ba0b40e88bd0dc26840172979889d refs/heads/dx2
Author: Burgholzer <Robert.Burgholzer@deq.virginia.gov>
Date:   Thu Mar 17 12:29:55 2022 -0400

    Added difficult run test json and roxy comments and next steps for full json object handling

commit af41bb205dca03c07ea12df46d8fc24731ab255b refs/remotes/origin/newsum2
Author: Burgholzer <robert.burgholzer@deq.virginia.gov>
Date:   Tue Jan 12 10:29:17 2021 -0500

    added new summaries
...

Then, using the commit "hash" (the long string of text after the word "commit"), I browsed to the commit in question on github, by appending the "hash" to the following URL: https://github.com/HARPgroup/om/commit/", so, for the last commit it would be https://github.com/HARPgroup/om/commit/892cbd124811310987a424688d515f0ecc338de7

Anyhow, I could quickly ascertain that nothing had been changed that would have omitted some old fix in the code for handling ps_mgd/ps_cumulative_mgd etc. (I looked ack to 2018 very quickly).

So, thanks a ton for finding the bug with Carvin's, I still think that we can check for ps_mgd and ps_cumulative_mgd, but am thinking we need to store a error code of some sort in the summaries so that we don't just gloss over the fact that the object was missing stuff that it should have had. Mentioning @COBrogan on this as we should think it all through. The overhaul needed for Carvin's is rather complicated as it pumps refill water from 2 separate streams, Tinker Creek and Catawba Creek (which crosses basin boundaries). https://github.com/HARPgroup/vahydro/issues/836

[rburghol]

This is fixed. Thanks again @glenncampagna for identifying this and getting us pointed along the way to a resolution. See PR #527