ConStrain Supply Air Temperature Reset Verification Measure #14
Conversation
jslane-h
commented
Sep 23, 2024
jslane-h
commented
- Model measure to create a supply air temperature reset verification case and workflow JSON to run that verification case
- Arguments are idf_path, output_dataset_path, output_dir, air_loop_name, design_zone_cooling_air_temp
- Saves constrain_workflow.json and supply_air_temperature_reset_verification_case.json in output_dir
…r_temperature_reset_verification
| design_zone_cooling_air_temp = runner.getDoubleArgumentValue( | ||
| "design_zone_cooling_air_temp", user_arguments | ||
| ) | ||
| idf_file_path = runner.getStringArgumentValue("idf_path", user_arguments) |
There was a problem hiding this comment.
@jslane-h - I feel like this is a bit confusing since this is an OpenStudio measure. Also, because we're not running the simulation using ConStrain I think this could be left blank in the verification case. If so, we could get rid of that argument. Have you tried it? If no, could you please give it a shot?
|
|
||
| measure.run(model, runner, argument_map) | ||
| result = runner.result() | ||
| assert result.value().valueName() == "Success" |
There was a problem hiding this comment.
Could we perhaps also check that the correct output variable has been added?
|
|
||
| args_dict = { | ||
| "air_loop_name": "Test Air Loop", | ||
| "design_zone_cooling_air_temp": 40, |
There was a problem hiding this comment.
I think this doesn't matter here but just for reference, let's change that to 24.
| args_dict = { | ||
| "air_loop_name": "Test Air Loop", | ||
| "design_zone_cooling_air_temp": 40, | ||
| "idf_path": "./lib/measures/ConstrainSupplyAirTemperatureResetVerification/tests/input/test.idf", |
There was a problem hiding this comment.
If we don't need that argument we should probably remove the IDF file from this PR.
| args.append(idf_path) | ||
| args.append(output_dataset_path) | ||
| args.append(output_dir) | ||
| # TODO: If the user doesn't provide a design_zone_cooling_air_temp, we can infer it from other modeling inputs |
There was a problem hiding this comment.
Maybe we should move that to a GitHub issue so it doesn't get buried?
There was a problem hiding this comment.
Thanks @jslane-h! I think that it overall looks good. I left a few comments. Have you run openstudio measure -t to see if any issue popped-up? We'll also have to update the license, it will probably be the same as the other measure but we can check.
|
Hi @kflemin - When I run In the past you pointed out that we should update our |
| model = openstudio.model.Model() | ||
|
|
||
| # get arguments | ||
| arguments = measure.arguments(model) |
There was a problem hiding this comment.
Why do you need to get arguments twice in the test function?
| air_loop, | ||
| design_zone_cooling_air_temp, | ||
| ) | ||
| with open(f"{output_dir}/supply_air_temperature_reset_verification_case.json", "w") as f: |
There was a problem hiding this comment.
Does the supply_air_temperature_reset_verification_case.json exists all the time?
If not, suggest adding a file exist check first.
There was a problem hiding this comment.
I added line 261 to handle this. open's "w" mode will create a new file if it doesn't exist, but it won't create any directories.
| json.dump(verification_cases, f, indent=2) | ||
|
|
||
| workflow = self.get_workflow(output_dataset_path, output_dir) | ||
| with open(f"{output_dir}/constrain_workflow.json", "w") as f: |
Hi @lymereJ, I see your change in .gitignore, looks good. The tests/output directory should not get committed to the repo since it contains temporary output files from running the tests. But you are getting an error when running locally, and i'm not sure why. If you delete the tests/output directory (or at least its contents) and run your local command again, do you get an error? If so, I would open a ticket on the OpenStudio repo. |
…ttps://github.com/pnnl/openstudio-building-energy-standard-measures-gem into constrain_supply_air_temperature_reset_verification
|
@kflemin - Thanks! Removing them did the trick. |
|
@weilixu - Have all your comments been addressed? |
