Add Integration Tests #326

nmerket · 2022-11-02T19:27:15Z

Fixes #318

Pull Request Description

This adds some integration tests where we checkout the lastest ResStock and run small versions of each of the testing projects in ResStock.

Checklist

Not all may apply

Code changes (must work)
Tests exercising your feature/bug fix (check coverage report on Checks -> BuildStockBatch Tests -> Artifacts)
Coverage has increased or at least not decreased. Update minimum_coverage in .github/workflows/ci.yml as necessary.
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

github-actions · 2022-11-02T19:49:44Z

File	Coverage
All files	`86%`	✅
base.py	`88%`	✅
eagle.py	`74%`	✅
exc.py	`57%`	✅
localdocker.py	`63%`	✅
postprocessing.py	`84%`	✅
utils.py	`96%`	✅
sampler/base.py	`79%`	✅
sampler/downselect.py	`33%`	✅
sampler/precomputed.py	`93%`	✅
sampler/residential_quota.py	`83%`	✅
test/test_validation.py	`96%`	✅
workflow_generator/base.py	`90%`	✅
workflow_generator/commercial.py	`53%`	✅
workflow_generator/residential.py	`96%`	✅
workflow_generator/residential_hpxml.py	`70%`	✅

Minimum allowed coverage is 33%

Generated by 🐒 cobertura-action against 563b9e7

This reverts commit 0febf50. [skip ci]

nmerket

My comments on what I did below. Merge #323 before this one.

nmerket · 2022-11-02T20:07:53Z

.github/workflows/ci.yml

+      - uses: actions/checkout@v3
+        with:
+          path: buildstockbatch
+      - uses: actions/checkout@v3
+        with:
+          repository: NREL/resstock
+          path: resstock
+          ref: develop


Checking out both repos

nmerket · 2022-11-02T20:08:08Z

.github/workflows/ci.yml

+          repository: NREL/resstock
+          path: resstock
+          ref: develop
+      - uses: actions/setup-python@v4


I generally tried to update to the latest versions of all the actions.

nmerket · 2022-11-02T20:08:29Z

.github/workflows/ci.yml

-on: [pull_request]
+on: 
+  push:
+    branches:
+      - develop
+  pull_request:
+    types:
+      - synchronize
+      - opened


Doing these checks on merge into develop as well on @shorowit's recommendation.

nmerket · 2022-11-02T20:09:07Z

.github/workflows/ci.yml

+      - name: Download weather
+        run: |
+          mkdir weather
+          cd weather
+          wget --quiet https://data.nrel.gov/system/files/156/BuildStock_TMY3_FIPS.zip


All the testing files use the same batch of weather files. This keeps it from being downloaded 4 times.

nmerket · 2022-11-02T20:09:45Z

buildstockbatch/test/test_integration.py

+@pytest.mark.parametrize("project_filename", [
+    resstock_directory / "project_national" / "national_baseline.yml",
+    resstock_directory / "project_national" / "national_upgrades.yml",
+    resstock_directory / "project_testing" / "testing_baseline.yml",
+    resstock_directory / "project_testing" / "testing_upgrades.yml",
+], ids=lambda x: x.stem)


I'm testing each of the project yaml files in resstock.

nmerket · 2022-11-02T20:10:59Z

buildstockbatch/test/test_integration.py

+    # Get the number of upgrades
+    n_upgrades = len(batch.cfg.get("upgrades", []))
+    # Limit the number of upgrades to 2 to reduce simulation time
+    if n_upgrades > 2:
+        batch.cfg["upgrades"] = batch.cfg["upgrades"][0:2]
+        n_upgrades = 2
+
+    # Modify the number of datapoints so we're not here all day.
+    if n_upgrades == 0:
+        n_datapoints = 4
+    else:
+        n_datapoints = 2
+    batch.cfg["sampler"]["args"]["n_datapoints"] = n_datapoints


I don't want to spend forever running these ResStock runs. I just want to make sure the machinery works, so I'm limiting the number of upgrades and datapoints to keep the CI running quickly.

nmerket · 2022-11-02T20:11:53Z

buildstockbatch/test/test_integration.py

+    local_weather_file = resstock_directory.parent / "weather" / batch.cfg["weather_files_url"].split("/")[-1]
+    if local_weather_file.exists():
+        del batch.cfg["weather_files_url"]
+        batch.cfg["weather_files_path"] = str(local_weather_file)


It'll check if the weather is found locally and swap that in instead. Since we downloaded it earlier to the appropriate place it should just be able to unzip it.

nmerket · 2022-11-02T20:12:39Z

create_eagle_env.sh

-conda create -y --prefix "$MY_CONDA_PREFIX" -c conda-forge "pyarrow>=7.0.0" "python=3.9" "numpy>=1.20.0" "pandas>=1.0.0,!=1.0.4" "dask>=2022.10.0" "distributed>=2021.5" ruby
-source deactivate 
-source activate "$MY_CONDA_PREFIX"
+mamba create -y --prefix "$MY_CONDA_PREFIX" -c conda-forge "python=3.10" "pyarrow" "numpy" "pandas" "dask>=2022.10.0" "distributed" "ruby"
+conda deactivate
+conda activate "$MY_CONDA_PREFIX"
 which pip
 if [ $DEV -eq 1 ]
 then
-    pip install --ignore-installed -e .
+    pip install --no-cache-dir --ignore-installed -e .
 else
-    pip install --ignore-installed .
+    pip install --no-cache-dir --ignore-installed .


These are changes from #323. I branched from that branch for this work. They will disappear once that is merged.

afontani · 2022-11-02T22:39:46Z

buildstockbatch/test/test_integration.py

+        assert (out_path / "results_csvs" / f"results_up{upgrade_id:02d}.csv.gz").exists()
+    assert (ts_pq_path / "_common_metadata").exists()
+    assert (ts_pq_path / "_metadata").exists()
+


Do all these asserts check to see if all the simulations failed? The machinery might work, but there should be some real results.

Good point. It does check that timeseries files exist for each simulation, but I will also add something to open the results parquet file and ensure they all succeeded.

Checking to see "all" succeeded seems robust, but we might accidentally sample a building that will fail in OpenStudio-HPXML. The current failure rate seems low (~0.1%) enough. At least 1 successful run might be okay.

joseph-robertson · 2022-11-03T18:20:26Z

CI time goes from about 3 min to 15 min here?

nmerket · 2022-11-04T18:00:14Z

CI time goes from about 3 min to 15 min here?

That's what happens when you actually start running simulations. I'm doing this for all versions of python we support. I may consider just doing it for the latest version, but it all happens in parallel so I don't see a problem with it.

nmerket · 2022-11-08T23:07:21Z

buildstockbatch/postprocessing.py

-                df.to_csv(gf, index=True, line_terminator='\n')
+                df.to_csv(gf, index=True, lineterminator='\n')


afontani

@nmerket : It looks like the tests are working. It makes sense that the CI time increased a little. I do like the parallelization. Unless you want to do anything else with this PR, I am happy with the functionality.

nmerket added 3 commits November 2, 2022 13:02

adding integration test for ResStock

446bdf4

including resstock checkout and some other things in ci

bb8c526

predownload the weather

fc668d1

pushing coverage minimum to 33

e7d40f8

afontani assigned nmerket Nov 2, 2022

nmerket added 4 commits November 2, 2022 13:56

reducing n_datapoints to make it take less time

3aceed4

ensuring that the weather files aren't downloaded for each test

0febf50

adding ci tests on push to develop

1c8de64

Revert "ensuring that the weather files aren't downloaded for each test"

fd5e22f

This reverts commit 0febf50. [skip ci]

nmerket commented Nov 2, 2022

View reviewed changes

nmerket requested review from joseph-robertson and afontani November 2, 2022 20:13

updating changelog [skip ci]

1735b9d

nmerket marked this pull request as ready for review November 2, 2022 20:16

removing resolved comment

5474050

nmerket mentioned this pull request Nov 2, 2022

Integration Tests for ComStock #327

Open

afontani reviewed Nov 2, 2022

View reviewed changes

Merge remote-tracking branch 'origin/develop' into integration-tests

3d128c6

checking results to ensure all simulations succeeded

8ad08fd

line_terminator is deprecated, using lineterminator instead

563b9e7

nmerket commented Nov 8, 2022

View reviewed changes

afontani self-requested a review November 8, 2022 23:49

afontani approved these changes Nov 8, 2022

View reviewed changes

nmerket merged commit 2757065 into develop Nov 10, 2022

nmerket deleted the integration-tests branch November 10, 2022 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Integration Tests #326

Add Integration Tests #326

nmerket commented Nov 2, 2022 •

edited

github-actions bot commented Nov 2, 2022 •

edited

nmerket left a comment

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

nmerket Nov 2, 2022

afontani Nov 2, 2022

nmerket Nov 2, 2022

afontani Nov 3, 2022

joseph-robertson commented Nov 3, 2022

nmerket commented Nov 4, 2022

nmerket Nov 8, 2022

afontani left a comment

		df.to_csv(gf, index=True, line_terminator='\n')
		df.to_csv(gf, index=True, lineterminator='\n')

Add Integration Tests #326

Add Integration Tests #326

Conversation

nmerket commented Nov 2, 2022 • edited

Pull Request Description

Checklist

github-actions bot commented Nov 2, 2022 • edited

nmerket left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joseph-robertson commented Nov 3, 2022

nmerket commented Nov 4, 2022

Choose a reason for hiding this comment

afontani left a comment

Choose a reason for hiding this comment

nmerket commented Nov 2, 2022 •

edited

github-actions bot commented Nov 2, 2022 •

edited