Adds method to process modelica results #646
Conversation
There was a problem hiding this comment.
Please run pre-commit and address the linting & formatting changes.
Overall it's not clear to me what this command is for. Will you add more context to the PR description (and perhaps a bit to the cli command description) explaining what the output is, why someone would use this command, and how to interpret it?
management/uo_des.py
Outdated
| :param start_time (int): start time of the simulation (seconds of a year) | ||
| :param stop_time (int): stop time of the simulation (seconds of a year) | ||
| :param step_size (int): step size of the simulation (seconds) | ||
| :param number_of_intervals (int): number of intervals to run the simulation |
There was a problem hiding this comment.
These are valid inputs to the run-model command, aren't they? Why remove them?
There was a problem hiding this comment.
corrected this
There was a problem hiding this comment.
These parameters are still valid, no? Why remove the documentation for them?
|
This needs to be incorporated |
what is the pre commit command? |
|
management/uo_des.py
Outdated
| @@ -170,28 +171,28 @@ def create_model(sys_param_file: Path, geojson_feature_file: Path, project_path: | |||
| @click.option( | |||
| "-a", | |||
| "--start_time", | |||
| default=17280000, | |||
| default=0, | |||
There was a problem hiding this comment.
Please explain why we need to change the default time settings for all CLI users.
management/uo_des.py
Outdated
| :param start_time (int): start time of the simulation (seconds of a year) | ||
| :param stop_time (int): stop time of the simulation (seconds of a year) | ||
| :param step_size (int): step size of the simulation (seconds) | ||
| :param number_of_intervals (int): number of intervals to run the simulation |
There was a problem hiding this comment.
These parameters are still valid, no? Why remove the documentation for them?
| # See also https://github.com/urbanopt/geojson-modelica-translator/blob/develop/LICENSE.md | ||
|
|
||
|
|
||
| import unittest |
There was a problem hiding this comment.
Please use the pytest setup that already exists in this repo, instead of unittest.
| modelica_path = modelica_path.resolve() | ||
|
|
||
| # Construct the path to the CSV file | ||
| csv_file_path = ( |
There was a problem hiding this comment.
If we already have output data from a modelica simulation, we don't need to change the default on the CLI, do we?
Those time options on the CLI are the specifically to allow users to set whatever timespan & timestep they require. For example, this test sets different values for those options, if you want to generate fresh data for a test.
There was a problem hiding this comment.
since we are already testing the run command it is sufficient to test the process command separately. I have also added a light weight model to this test.
Objective :
This cli call takes the modelica folder created and run previously as an argument. It then reads the .mat file and queries timeseries results for heating, cooling electricity consumption of heat pumps, ets pump electricity consumption, pump associated with building electricity consumption loop pump electricity consumption and heat pump sizing for the GHP network.
This uses methods from the modelica buildingspy library to query results. https://simulationresearch.lbl.gov/modelica/buildingspy/
To test :
Use
uo_des des-process<modelica_model_name>`Run
poetry run pytest tests poetry run pytest .\tests\geojson_modelica_translator\test_results_ghp.pyCheck the "geojson-modelica-translator\tests\geojson_modelica_translator\data\modelica_5\modelica_5.Districts.DistrictEnergySystem_results\modelica_5.Districts.DistrictEnergySystem_result.csv" file created with results queried for REopt GHP at 15 minute intervals.