search

EnergyInnovation / eps-us

Energy Policy Simulator - United States
GNU General Public License v3.0
22 stars 7 forks source link

Data Logging Script adding Time rows within a single variable output #86

Closed robbieorvis closed 4 years ago

robbieorvis commented 4 years ago

The data logging script appears to be adding some extra Time lines for certain variables when saving out data. See attached for an example DataLoggingExample.xlsx

jrissman commented 4 years ago

It looks like this is happening in bldgs/BCEU because the variable is of mixed type: most of the subscript elements are data type, populated by GET DIRECT DATA, but the "envelope" subscript element is a constant type, because it has a hard-coded zero in Vensim. Adding the extra "time" rows is the behavior of Vensim's VDF2TAB function when operating on variables of mixed type - it isn't an issue with the Python script.

I can fix this by reading in a CSV file for "envelope" for BCEU that is filled with zeroes. Then BCEU would not be of mixed type anymore - the variable would be entirely data type. On the downside, we'd have an extra blue tab in that Excel file, and an extra CSV file to read in, which should always contain only zeroes. I assume you think it's better to have the extra blue tab and extra CSV file full of zeroes in order to avoid the extra time rows in the script output, right?

Are there any other variables besides BCEU where you have seen this issue? I need to fix each variable individually, so I'd need to know which ones are of mixed type. There might not be any others besides BCEU - it's pretty rare for us to be taking in data from CSV files for some subscript elements and hard-code others. The handling of "envelope" in the buildings sector is unique, or close to unique.

jrissman commented 4 years ago

Also, it only applies to the first variable to be listed, because I'm suppressing the time axis on all variables after the first. If you don't need any time axis at all, even on the first variable, I could suppress it there too. But I'm reluctant to do that because it can be a very important way to catch bugs - for instance, you might not notice time mis-alignments if there is no time axis showing. So I'd recommend against the approach of suppressing the time axis entirely.

robbieorvis commented 4 years ago

Interesting.

Haven’t seen it elsewhere, so far.

I guess if it’s just here, then yes it would be helpful to add the .csv file. Otherwise it comes up every time we create a calibration sheet (and update the calibration sheet data).

Robbie Orvis Director of Energy Policy Design Phone: 415-799-2171 98 Battery Street, Suite 202 San Francisco, CA 94111 www.energyinnovation.orghttp://www.energyinnovation.org/ [cid:image001.jpg@01D0D699.20A24470]

Check out our new book, Designing Climate Solutions: A Policy Guide for Low-Carbon Energyhttps://www.amazon.com/Designing-Climate-Solutions-Policy-Low-Carbon/dp/1610919564 Available wherever books are sold

[Policy Design book cover]

From: Jeff Rissman notifications@github.com Sent: Wednesday, August 12, 2020 2:57 PM To: Energy-Innovation/eps-us eps-us@noreply.github.com Cc: Robbie Orvis robbie@energyinnovation.org; Author author@noreply.github.com Subject: Re: [Energy-Innovation/eps-us] Data Logging Script adding Time rows within a single variable output (#86)

It looks like this is happening in bldgs/BCEU because the variable is of mixed type: most of the subscript elements are data type, populated by GET DIRECT DATA, but the "envelope" subscript element is a constant type, because it has a hard-coded zero in Vensim. Adding the extra "time" rows is the behavior of Vensim's VDF2TAB function when operating on variables of mixed type - it isn't an issue with the Python script.

I can fix this by reading in a CSV file for "envelope" for BCEU that is filled with zeroes. Then BCEU would not be of mixed type anymore - the variable would be entirely data type. On the downside, we'd have an extra blue tab in that Excel file, and an extra CSV file to read in, which should always contain only zeroes. I assume you think it's better to have the extra blue tab and extra CSV file full of zeroes in order to avoid the extra time rows in the script output, right?

Are there any other variables besides BCEU where you have seen this issue? I need to fix each variable individually, so I'd need to know which ones are of mixed type. There might not be any others besides BCEU - it's pretty rare for us to be taking in data from CSV files for some subscript elements and hard-code others. The handling of "envelope" in the buildings sector is unique, or close to unique.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/Energy-Innovation/eps-us/issues/86#issuecomment-673130723, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK5N6SKJY7RZOC5EYCY6GIDSAMF35ANCNFSM4P5K2HKQ.

robbieorvis commented 4 years ago

No, let’s keep it for the first, just want to avoid that weird repeating behavior.

Robbie Orvis Director of Energy Policy Design Phone: 415-799-2171 98 Battery Street, Suite 202 San Francisco, CA 94111 www.energyinnovation.orghttp://www.energyinnovation.org/ [cid:image001.jpg@01D0D699.20A24470]

Check out our new book, Designing Climate Solutions: A Policy Guide for Low-Carbon Energyhttps://www.amazon.com/Designing-Climate-Solutions-Policy-Low-Carbon/dp/1610919564 Available wherever books are sold

[Policy Design book cover]

From: Jeff Rissman notifications@github.com Sent: Wednesday, August 12, 2020 3:00 PM To: Energy-Innovation/eps-us eps-us@noreply.github.com Cc: Robbie Orvis robbie@energyinnovation.org; Author author@noreply.github.com Subject: Re: [Energy-Innovation/eps-us] Data Logging Script adding Time rows within a single variable output (#86)

Also, it only applies to the first variable to be listed, because I'm suppressing the time axis on all variables after the first. If you don't need any time axis at all, even on the first variable, I could suppress it there too. But I'm reluctant to do that because it can be a very important way to catch bugs - for instance, you might not notice time mis-alignments if there is no time axis showing. So I'd recommend against the approach of suppressing the time axis entirely.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/Energy-Innovation/eps-us/issues/86#issuecomment-673131751, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK5N6SPPV7OF2X6STK4DZ3LSAMGHJANCNFSM4P5K2HKQ.

jrissman commented 4 years ago

Completed in commit 175d793.

robbieorvis commented 4 years ago

Alright I lied, it’s happening in select other places in the file. If you run it and then search for Time you’ll see. Try using this output vars file.

Robbie Orvis Director of Energy Policy Design Phone: 415-799-2171 98 Battery Street, Suite 202 San Francisco, CA 94111 www.energyinnovation.orghttp://www.energyinnovation.org/ [cid:image001.jpg@01D0D699.20A24470]

Check out our new book, Designing Climate Solutions: A Policy Guide for Low-Carbon Energyhttps://www.amazon.com/Designing-Climate-Solutions-Policy-Low-Carbon/dp/1610919564 Available wherever books are sold

[Policy Design book cover]

From: Jeff Rissman notifications@github.com Sent: Wednesday, August 12, 2020 2:57 PM To: Energy-Innovation/eps-us eps-us@noreply.github.com Cc: Robbie Orvis robbie@energyinnovation.org; Author author@noreply.github.com Subject: Re: [Energy-Innovation/eps-us] Data Logging Script adding Time rows within a single variable output (#86)

It looks like this is happening in bldgs/BCEU because the variable is of mixed type: most of the subscript elements are data type, populated by GET DIRECT DATA, but the "envelope" subscript element is a constant type, because it has a hard-coded zero in Vensim. Adding the extra "time" rows is the behavior of Vensim's VDF2TAB function when operating on variables of mixed type - it isn't an issue with the Python script.

I can fix this by reading in a CSV file for "envelope" for BCEU that is filled with zeroes. Then BCEU would not be of mixed type anymore - the variable would be entirely data type. On the downside, we'd have an extra blue tab in that Excel file, and an extra CSV file to read in, which should always contain only zeroes. I assume you think it's better to have the extra blue tab and extra CSV file full of zeroes in order to avoid the extra time rows in the script output, right?

Are there any other variables besides BCEU where you have seen this issue? I need to fix each variable individually, so I'd need to know which ones are of mixed type. There might not be any others besides BCEU - it's pretty rare for us to be taking in data from CSV files for some subscript elements and hard-code others. The handling of "envelope" in the buildings sector is unique, or close to unique.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/Energy-Innovation/eps-us/issues/86#issuecomment-673130723, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK5N6SKJY7RZOC5EYCY6GIDSAMF35ANCNFSM4P5K2HKQ.

jrissman commented 4 years ago

I think you forgot to attach the output vars file. Or at least, the attachment didn't make it through to GitHub.

robbieorvis commented 4 years ago

Here you go. outputexample.xlsx

jrissman commented 4 years ago

Okay, Vensim's behavior with including multiple Time rows is buggier than I thought. It doesn't just apply to variables of mixed type, like BCEU. It is not clear why Vensim includes Time rows for six of the variables in that output but not the others.

To work around this, I've modified the script so that we suppress the time row entirely from Vensim's VDF2TAB output. So that you still have a Time row, we now generate the tab-separated values output file when running the Python script and build the Time row using Python. Then you run the command script, and Vensim appends the run data to the .tsv file, rather than overwriting the file. This results in a clean output file with a single Time row at the top.

Completed in commit b5e5f12.