Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Two small fixes #424

Merged
merged 2 commits into from
Jan 5, 2024
Merged

Two small fixes #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9f9532
Two small fixes
nweires Dec 11, 2023
1b60710
Update test
nweires Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion buildstockbatch/aws/aws.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,7 +1723,7 @@ def run_job(cls, job_id, bucket, prefix, job_name, region):
logger.debug("Extracting {}".format(epw_filename))
f_out.write(gzip.decompress(f_gz.getvalue()))

cls.run_simulations(cfg, jobs_d, job_id, sim_dir, S3FileSystem(), bucket, prefix)
cls.run_simulations(cfg, jobs_d, job_id, sim_dir, S3FileSystem(), f"{bucket}/{prefix}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that in the run_simulations function definition there's just an output_path argument. Does that mean this wasn't already working?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the function params in response to this comment, but missed updating this line.

Meanwhile, I'm also looking into why the test didn't catch this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments to test_docker_base.py to at least explain why the test there won't catch issues like the one in docker_base.py.

We should try to get that test to test things more thoroughly, but that's more involved so I'll save it for a separate PR.



@log_error_details()
Expand Down
2 changes: 1 addition & 1 deletion buildstockbatch/cloud/docker_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def run_simulations(cls, cfg, job_id, jobs_d, sim_dir, fs, output_path):
dpouts = []
simulation_output_tar_filename = sim_dir.parent / "simulation_outputs.tar.gz"
asset_dirs = os.listdir(sim_dir)
ts_output_dir = (f"{output_path}/results/simulation_output/timeseries",)
ts_output_dir = f"{output_path}/results/simulation_output/timeseries"

with tarfile.open(str(simulation_output_tar_filename), "w:gz") as simout_tar:
for building_id, upgrade_idx in jobs_d["batch"]:
Expand Down
15 changes: 15 additions & 0 deletions buildstockbatch/test/test_docker_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for the DockerBatchBase class."""
from fsspec.implementations.local import LocalFileSystem
import gzip
import json
import os
import pathlib
Expand Down Expand Up @@ -97,6 +98,12 @@ def test_get_epws_to_download():


def test_run_simulations(basic_residential_project_file):
"""
Test running a single batch of simulation.

This doesn't provide all the necessary inputs for the simulations to succeed, but it confirms that they are
attempted, that the output files are produced, and that intermediate files are cleaned up.
"""
jobs_d = {
"job_num": 0,
"n_datapoints": 10,
Expand Down Expand Up @@ -124,6 +131,14 @@ def test_run_simulations(basic_residential_project_file):

output_dir = bucket / "test_prefix" / "results" / "simulation_output"
assert sorted(os.listdir(output_dir)) == ["results_job0.json.gz", "simulations_job0.tar.gz"]

# Check that buildings 1 and 5 (specified in jobs_d) are in the results
with gzip.open(output_dir / "results_job0.json.gz", "r") as f:
results = json.load(f)
assert len(results) == 2
for building in results:
assert building["building_id"] in (1, 5)

# Check that files were cleaned up correctly
assert not os.listdir(sim_dir)
os.chdir(old_cwd)
Loading