Append schedules to timeseries #186

rajeee · 2020-10-05T18:39:00Z

Appends schedules to the timeseries output so that it gets aggregated.
DST time-shifting adjusted during append.

Pull Request Description

description here

Checklist

Not all may apply

Code changes (must work)
Tests exercising your feature/bug fix (check coverage report on CircleCI build -> Artifacts)
All other unit tests passing
Update validation for project config yaml file changes
Update existing documentation
Run a small batch run to make sure it all works (local is fine, unless an Eagle specific feature)
Add to the changelog_dev.rst file and propose migration text in the pull request

nmerket

I like this approach better than the previous one. See my comments below.

Also, make sure you're going through the checklist in the PR (tests, docs, etc).

nmerket · 2020-10-06T17:32:03Z

buildstockbatch/base.py

        if os.path.isfile(timeseries_filepath):
-            tsdf = pd.read_csv(timeseries_filepath, parse_dates=[0])
+            tsdf = pd.read_csv(timeseries_filepath, parse_dates=['Time', 'TimeDST', 'TimeUTC'])
+            schedules = pd.read_csv(schedules_filepath)


What if the simulation doesn't generate a schedules.csv? It looks like you'll get a file not found error. There are going to continue to be versions of OpenStudio-BuildStock that don't generate schedules.csv. This should be able to gracefully handle that.

Hmm, good point. I will add handling for that.

buildstockbatch/base.py

nmerket · 2020-10-06T17:36:53Z

buildstockbatch/base.py

+            schedules['TimeDST'] = tsdf['Time']
+            ts_and_schedule = tsdf.merge(schedules, how='left', on='TimeDST')


What happens to the extra hour here? Is it filled with NaN? Is that what you want?

The value for 1st hour at the beginning of the daylight saving period will be discarded, and the one at the end will be duplicated. This is consistent with how Energy+ handles scheduleFiles when daylight saving time is active.

Co-authored-by: Noel Merket <noel.merket@nrel.gov>

nmerket

A couple things:

Do we really need all these extra files in the test folder for this feature?
It looks like the tests are failing.

nmerket · 2020-10-13T18:08:42Z

buildstockbatch/test/test_eagle.py

+        compare_ts_parquets(file, results_file)
+
+    # Hack to clear the temporary directory.
+    subprocess.run(['rm', '-r', results_dir.parent.as_posix()])


This will only work on Linux/Mac. Windows doesn't have the posix rm command. Should use something like shutil.rmtree for a more "python native" version.

Ideally, we shouldn't even need this hack because once you exit the fixture, the temporary directory should be automatically be cleared. I am trying to figure out what's going on.
shutil.rmtree didn't work on my laptop, so I tried to use rm -r. Something really weird going on.

rajeee · 2020-10-14T23:22:08Z

A couple things:

Do we really need all these extra files in the test folder for this feature?

It looks like the tests are failing.

We don't need all of them for this particular test, but, I thought those would be a good reference set of test files to have since they are what the raw result from simulation looks like. So, they will come in handy to test any feature (now or in future) that processes the raw result.
Working on it.

nmerket · 2020-10-19T16:05:31Z

A couple things:

Do we really need all these extra files in the test folder for this feature?

It looks like the tests are failing.

1. We don't need all of them for this particular test, but, I thought those would be a good reference set of test files to have since they are what the raw result from simulation looks like. So, they will come in handy to test any feature (now or in future) that processes the raw result.

2. Working on it.

It's just a lot of files that we aren't using, though. Let's remove anything from that simulations_job0 folder that isn't used for the tests.

…ch into schedule_aggregation2

rajeee · 2020-10-19T21:36:22Z

A couple things:

Do we really need all these extra files in the test folder for this feature?

It looks like the tests are failing.
1. We don't need all of them for this particular test, but, I thought those would be a good reference set of test files to have since they are what the raw result from simulation looks like. So, they will come in handy to test any feature (now or in future) that processes the raw result.

2. Working on it.
It's just a lot of files that we aren't using, though. Let's remove anything from that simulations_job0 folder that isn't used for the tests.

@nmerket Removed files as suggested.

nmerket

Looks good!

Append schedules to timeseries

36d3d27

rajeee requested review from nmerket and afontani October 5, 2020 18:39

nmerket reviewed Oct 6, 2020

View reviewed changes

rajeee and others added 3 commits October 7, 2020 16:27

Update buildstockbatch/base.py

7f1ce0d

Co-authored-by: Noel Merket <noel.merket@nrel.gov>

Add test for raw simulation results processing

ad016dc

Adding missing files

a0f5811

rajeee requested a review from nmerket October 12, 2020 15:02

nmerket reviewed Oct 13, 2020

View reviewed changes

rajeee added 3 commits October 14, 2020 17:55

Add call to create directories

50d7104

Fix style error

2682037

Remove the problematic patch

ccd44b9

rajeee requested a review from nmerket October 15, 2020 15:54

rajeee added 3 commits October 15, 2020 09:54

Merge branch 'develop' into schedule_aggregation2

c8ec4c9

Update documentation and changelog

7ad416c

Fix formatting error for preject_defn.rst.

87fa2b0

rajeee added 2 commits October 19, 2020 15:30

remove unnecessary test files

f72ce52

Merge branch 'schedule_aggregation2' of github.com:NREL/buildstockbat…

d8a320c

…ch into schedule_aggregation2

nmerket approved these changes Oct 21, 2020

View reviewed changes

Merge branch 'develop' into schedule_aggregation2

9c2c3cd

rajeee merged commit f17c57a into develop Oct 21, 2020

rajeee deleted the schedule_aggregation2 branch October 21, 2020 23:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Append schedules to timeseries #186

Append schedules to timeseries #186

rajeee commented Oct 5, 2020 •

edited

nmerket left a comment

nmerket Oct 6, 2020

rajeee Oct 7, 2020

nmerket Oct 6, 2020 •

edited

rajeee Oct 7, 2020

nmerket left a comment

nmerket Oct 13, 2020

rajeee Oct 14, 2020

rajeee commented Oct 14, 2020

nmerket commented Oct 19, 2020

rajeee commented Oct 19, 2020

nmerket left a comment

		schedules['TimeDST'] = tsdf['Time']
		ts_and_schedule = tsdf.merge(schedules, how='left', on='TimeDST')

Append schedules to timeseries #186

Append schedules to timeseries #186

Conversation

rajeee commented Oct 5, 2020 • edited

Pull Request Description

Checklist

nmerket left a comment

Choose a reason for hiding this comment

nmerket Oct 6, 2020

Choose a reason for hiding this comment

rajeee Oct 7, 2020

Choose a reason for hiding this comment

nmerket Oct 6, 2020 • edited

Choose a reason for hiding this comment

rajeee Oct 7, 2020

Choose a reason for hiding this comment

nmerket left a comment

Choose a reason for hiding this comment

nmerket Oct 13, 2020

Choose a reason for hiding this comment

rajeee Oct 14, 2020

Choose a reason for hiding this comment

rajeee commented Oct 14, 2020

nmerket commented Oct 19, 2020

rajeee commented Oct 19, 2020

nmerket left a comment

Choose a reason for hiding this comment

rajeee commented Oct 5, 2020 •

edited

nmerket Oct 6, 2020 •

edited