Conversation
|
I've read through this line by line and would like to checkout this branch to use it for a while as integration tests before approving. Thanks for handling the refactor of Update: based on the discussion with @rouille over the phone, to clarify, we can't use this branch to run scenarios until we manually fix the ScenarioList.csv on the server by adding a "grid model" column at the end. The reason I would like to check it out for now is to use updated |
| def test_interconnect_type(): | ||
| interconnect = "Western" | ||
| with pytest.raises(TypeError): | ||
| check_interconnect(interconnect) |
There was a problem hiding this comment.
Is this showing an unnecessarily strict behavior for check_interconnect? We can now instantiate a Grid using Grid("Western"), are there other places where we do this check before doing something that really needs a list rather than a string?
There was a problem hiding this comment.
Looking where the Check_interconnect is used, I would say we can implement the:
if isinstance(interconnect, str):
interconnect = [interconnect]
inside the function and instantiate a ModelImmutables class in the constructor of the Grid class to check the interconnect. What do you think?
There was a problem hiding this comment.
I like that code pattern that gives us a bit more flexibility on the inputs when the intent is unambiguous.
There was a problem hiding this comment.
I will refactor it then
There was a problem hiding this comment.
This is addressed in the last two commits.
There was a problem hiding this comment.
Also, you can do that now:
(PowerSimData) [~/CEM/PowerSimData] (ben/create) brdo$ python
Python 3.8.3 (default, Jul 8 2020, 14:27:55)
[Clang 11.0.3 (clang-1103.0.32.62)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from powersimdata.input.grid import Grid
>>> g = Grid(["Western"])
Reading bus.csv
Reading plant.csv
Reading gencost.csv
Reading branch.csv
Reading dcline.csv
Reading sub.csv
Reading bus2sub.csv
Reading zone.csv
>>> g.model_immutables.zones.keys()
dict_keys(['abv', 'abv2interconnect', 'abv2loadzone', 'abv2state', 'id2abv', 'id2loadzone', 'id2timezone', 'interconnect', 'interconnect2abv', 'interconnect2id', 'interconnect2loadzone', 'interconnect2timezone', 'interconnect_combinations', 'loadzone', 'loadzone2id', 'loadzone2interconnect', 'loadzone2state', 'mappings', 'state', 'state2abv', 'state2loadzone', 'timezone2id'])
There was a problem hiding this comment.
It does not work right now when we try to instantiate the Grid class from a file since we don't have access to the grid_model value in the ScenarioList file:
>>> from powersimdata.scenario.scenario import Scenario
>>> s = Scenario(1712)
Failed to download ScenarioList.csv from server
Falling back to local cache...
Failed to download ExecuteList.csv from server
Falling back to local cache...
SCENARIO: test | new_bus
--> State
analyze
--> Loading grid
Failed to download ScenarioList.csv from server
Falling back to local cache...
Traceback (most recent call last):
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 33, in __init__
self.model_immutables = ModelImmutables(source)
File "/Users/brdo/CEM/PowerSimData/powersimdata/network/model.py", line 11, in __init__
self._check_model(model)
File "/Users/brdo/CEM/PowerSimData/powersimdata/network/model.py", line 31, in _check_model
raise ValueError("model must be one of %s" % " | ".join(possible))
ValueError: model must be one of usa_tamu
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexes/base.py", line 3080, in get_loc
return self._engine.get_loc(casted_key)
File "pandas/_libs/index.pyx", line 70, in pandas._libs.index.IndexEngine.get_loc
File "pandas/_libs/index.pyx", line 101, in pandas._libs.index.IndexEngine.get_loc
File "pandas/_libs/hashtable_class_helper.pxi", line 4554, in pandas._libs.hashtable.PyObjectHashTable.get_item
File "pandas/_libs/hashtable_class_helper.pxi", line 4562, in pandas._libs.hashtable.PyObjectHashTable.get_item
KeyError: 'grid_model'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/scenario.py", line 40, in __init__
self.state = Analyze(self)
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/analyze.py", line 39, in __init__
self._set_ct_and_grid()
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/analyze.py", line 50, in _set_ct_and_grid
self.grid = Grid(
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 36, in __init__
_get_grid_model_from_scenario_list(source)
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 165, in _get_grid_model_from_scenario_list
return slm.get_scenario_table().loc[scenario_number, "grid_model"]
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 888, in __getitem__
return self._getitem_tuple(key)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 1059, in _getitem_tuple
return self._getitem_lowerdim(tup)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 830, in _getitem_lowerdim
return getattr(section, self.name)[new_key]
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 894, in __getitem__
return self._getitem_axis(maybe_callable, axis=axis)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 1123, in _getitem_axis
return self._get_label(key, axis=axis)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexing.py", line 1072, in _get_label
return self.obj.xs(label, axis=axis)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/generic.py", line 3736, in xs
loc = index.get_loc(key)
File "/Users/brdo/.local/share/virtualenvs/PowerSimData-MpUK62nT/lib/python3.8/site-packages/pandas/core/indexes/base.py", line 3082, in get_loc
raise KeyError(key) from err
KeyError: 'grid_model'
| """ | ||
| scenario_number = int(os.path.basename(source).split("_")[0]) | ||
| slm = ScenarioListManager(Context.get_data_access()) | ||
| return slm.get_scenario_table().loc[scenario_number, "grid_model"] |
There was a problem hiding this comment.
You can also use slm.get_scenario(scenario_number)["grid_model"] (no real difference, just FYI)
There was a problem hiding this comment.
>>> from powersimdata.scenario.scenario import Scenario
>>> s = Scenario(1712)
Failed to download ScenarioList.csv from server
Falling back to local cache...
Failed to download ExecuteList.csv from server
Falling back to local cache...
SCENARIO: test | new_bus
--> State
analyze
--> Loading grid
Failed to download ScenarioList.csv from server
Falling back to local cache...
Traceback (most recent call last):
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 33, in __init__
self.model_immutables = ModelImmutables(source)
File "/Users/brdo/CEM/PowerSimData/powersimdata/network/model.py", line 11, in __init__
self._check_model(model)
File "/Users/brdo/CEM/PowerSimData/powersimdata/network/model.py", line 31, in _check_model
raise ValueError("model must be one of %s" % " | ".join(possible))
ValueError: model must be one of usa_tamu
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/scenario.py", line 40, in __init__
self.state = Analyze(self)
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/analyze.py", line 39, in __init__
self._set_ct_and_grid()
File "/Users/brdo/CEM/PowerSimData/powersimdata/scenario/analyze.py", line 50, in _set_ct_and_grid
self.grid = Grid(
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 36, in __init__
_get_grid_model_from_scenario_list(source)
File "/Users/brdo/CEM/PowerSimData/powersimdata/input/grid.py", line 165, in _get_grid_model_from_scenario_list
return slm.get_scenario(scenario_number)["grid_model"]
KeyError: 'grid_model'
Somehow the Traceback is smaller with this implementation. Not sure why ....
Pull Request Etiquette doc
Purpose
Allow the user to specify the grid model he wants to use in his scenario and access the corresponding profiles.
What the code is doing
It instantiates the
_Builderclass with an additional parametergrid_model. To use theusa_tamugrid model for theWesterninterconnect, the user will call theset_buildermethod in theCreateclass as follows:By default
set_builderwill instantiate theBuilderclass for the full U.S. TAMU grid.Testing
The following script:
prints the expected outputs:
Note that I did not create a scenario since the ScenarioList.csv is not yet formatted to handle the extra
grid_modelinformation. This will be the next step.Where to look
ModelImmutablesclass inpowersimdata.network.model: abstract the grid modelpowersimdata.network.usa_tamu.usa_tamu_modelaspowersimdata.network.usa_tamu.modeland change import statements.are_to_loadzonefunction inpowersimdata.network.usa_tamu.model: remove dependency on the aGridobjectarea_to_loadzoneinterconnect_to_namefunction inpowersimdata.network.usa_tamu.model: convert a list of interconnect(s) to a string that will be written in the interconnect column of the ScenarioList.csv, e.g.["Western", "texas"]--> "Texas_Western"usa_tamurelated tests inpowersimdata.input.tests.test_gridinto a newpowersimdata.network.usa_tamu.tests.test_modelget_profile_versionmethod ofInputDataclassdirin sub-modulespowersimdata.network.usa_tamu.constantsfollowing PEP 562 to allow easy look up inModelImmutablesclasscopy_base_profileof theBackupDiskclass inpowersimdata.scenario.movemodule_create_link_to_base_profileand_prepare_transformed_profilemethods of theSimulationInputclass inpowersimdata.scenario.executemodule. Refactor the_prepare_profileone. The reason is that we cannot create link to base profiles because the profile being normalized, we need to scale the values by the nameplate capacity of the plants at the very least. We could still create a symbolic link to the demand profiles but these being small I decided to keep the code simple and make no distinction among profiles.Createand_Builderclasses inpowersimdata.scenario.createmodule. The module has been greatly simplified.powersimdata.scenario.helpersmoduleTime estimate
It will be long