Skip to content

Refactor Scaler Class#186

Merged
rouille merged 15 commits intodevelopfrom
scaler_refactor
May 27, 2020
Merged

Refactor Scaler Class#186
rouille merged 15 commits intodevelopfrom
scaler_refactor

Conversation

@rouille
Copy link
Copy Markdown
Collaborator

@rouille rouille commented May 21, 2020

Purpose

This PR accomplishes three things:

  • refactor the Scaler class. There are now two classes, TransformGrid and ScaleProfile (Create TransformGrid and ScaleProfile Class #189).
    • The TransformGrid class is instantiated via a Grid object and a change table and return a Grid object via the get_grid method that has been transformed according to the changes listed in the change table. Transformations incliude scaling opertations and additions of storage units and AC/DC lines. The object is fully tested.
    • The ScaleProfile class is instantiated a paramiko.client.SSHClient object, a scenario id, a Grid object and a change table. The first two parameters are needed to download the base profiles from the server. The Grid object is needed to retrieve the plant id of a given type in a zone. The get_demand and get_power_output return a scaled profile using information in the change table for demand and clean energy generators, respectfully. Integration tests have been developed to test the scaling procedure.
  • A Grid object and a change table are now returned via the get_grid and get_ct methods of the Create class. If a Scenario object is in the execute state, scenario.state.get_grid() and scenario.state.get_ct() will return a transformed Grid object and the change table for this Scenario object (Get grid in the create and execute states #190).
  • fix a bug in the Analyze class (Profiles are not returned properly for a scenario with fewer solar/wind plants #169). Profiles are now returned using a Grid object coming from the instantiation of the Grid class via the input.mat. Also, the class has now a grid and ct attributes.

Where to look?

  • powersimdata.input.change_table.py. The ChangeTable class is noow instantiated via a Grid object. Previously, the constructor was taking a list of interconnect(s) and the Grid class was instantiated within the constructor of the ChangeTable class. Doing this, we avoid multiple instantiation of the ChangeTable class in the simulation framework since the same grid can be used to create a ChangeTable, TransformGrid and ScaleProfile objects.
  • powersimdata.input.scaler.py. Full refactor. The Scaler class has been replaced by the TransformGrid and ScaleProfile classes.
  • powersimdata.input.tests.test_transform_grid.py. Test the TransformGrid class.
  • powersimdata.input.tests.test_scale_profile.py. Test the ScaleProfile class. These are integration tests that are very slow.
  • powersimdata.input.tests.test_grid.py. Remove unused unittest package
  • pytest.ini. Define the integration marker. Using the -m integration option in the pytest command, only integration tests will be run. The or -m 'not integration' option will not run the integration tests. Pytest will automatically detect the file.
  • powersimdata.scenario.create.py. The _Builder class has now a get_grid method. The Create class has now a grid and ct attributes that will be carried over when swithing from the create to the execute state.
  • powersimdata.scenario.execute.py. When instantiated, a grid and ct attributes are set. A ScaleProfile object is used in the SimulationInput class.
  • powersimdata.scenario.analyze.py. The Grid class is now instantiated only via the input.mat. A grid and ct attributes are set during instantiation.
  • powersimdata.scenario.state.py. Delete the grid and ct attributes when switching from the analyze to delete state.

Visuals

I have placed the test_scenario_framework.ipynb notebook that you can look at (change the scenario name if you want to run it since the scenario has not been deleted). It performs a full integration test with checks every step of the way.

Time estimate

3h. It is long, sorry about that.

Tests

All tests pass.

(v1) [~/REM/PowerSimData] (scaler_refactor) brdo$ pytest .
============================================================================================== test session starts ==============================================================================================
platform darwin -- Python 3.6.5, pytest-4.5.0, py-1.8.0, pluggy-0.13.0
rootdir: /Users/brdo/REM/PowerSimData, inifile: pytest.ini
collected 146 items                                                                                                                                                                                             

powersimdata/design/tests/test_object_persistence.py ...                                                                                                                                                  [  2%]
powersimdata/design/tests/test_resource_target_manager.py ...............                                                                                                                                 [ 12%]
powersimdata/design/tests/test_scenario_info.py ........                                                                                                                                                  [ 17%]
powersimdata/design/tests/test_strategies.py ................                                                                                                                                             [ 28%]
powersimdata/design/tests/test_target_manager_input.py ...                                                                                                                                                [ 30%]
powersimdata/design/tests/test_transmission.py .....................................                                                                                                                      [ 56%]
powersimdata/input/tests/test_change_table.py ..........                                                                                                                                                  [ 63%]
powersimdata/input/tests/test_grid.py ............................                                                                                                                                        [ 82%]
powersimdata/input/tests/test_scale_profile.py ....                                                                                                                                                       [ 84%]
powersimdata/input/tests/test_transform_grid.py ...........                                                                                                                                               [ 92%]
powersimdata/tests/test_mocks.py ......                                                                                                                                                                   [ 96%]
powersimdata/utility/tests/test_transfer_data.py .....                                                                                                                                                    [100%]

========================================================================================== 146 passed in 36.62 seconds ==========================================================================================

@rouille rouille added the new feature Feature that is currently in progress. label May 21, 2020
@rouille rouille added this to the Glorious Sun milestone May 21, 2020
@rouille rouille requested review from BainanXia and danielolsen May 21, 2020 17:07
@danielolsen
Copy link
Copy Markdown
Contributor

fix a bug in the Analyze class

Is this #169?

@rouille rouille force-pushed the scaler_refactor branch 2 times, most recently from 1bad06b to db6db25 Compare May 21, 2020 18:50
@rouille rouille added bug Something isn't working grid refactor Code that is being refactored labels May 21, 2020
@rouille rouille changed the title Scaler refactor Refactor Scaler Class May 21, 2020
@danielolsen
Copy link
Copy Markdown
Contributor

I can confirm that this solves #169. Using 35cc7d0 (before new Western & Texas gens):

>>> from powersimdata.scenario.scenario import Scenario
>>> Scenario('462').state.get_solar().sum().sum()
SCENARIO: base | USABase_2016_new_HVDC_2

--> State
analyze
100%|#####################################| 7.25k/7.25k [00:00<00:00, 72.4kb/s]
--> Loading solar
462_solar.csv not found in C:\Users\dolsen\ScenarioData\ on local machine
Transferring 462_solar.csv from server
100%|#######################################| 104M/104M [00:07<00:00, 14.5Mb/s]
51111429.916688666

Using 1202b64 (after the new Western & Texas gens):

>>> from powersimdata.scenario.scenario import Scenario
>>> Scenario('462').state.get_solar().sum().sum()
SCENARIO: base | USABase_2016_new_HVDC_2

--> State
analyze
100%|#####################################| 7.25k/7.25k [00:00<00:00, 73.2kb/s]
--> Loading solar
462_solar.csv not found in C:\Users\dolsen\ScenarioData\ on local machine
Transferring 462_solar.csv from server
100%|#######################################| 104M/104M [00:06<00:00, 14.9Mb/s]
48714264.31347291

Using this branch rebased onto 1202b64:

>>> Scenario('462').state.get_solar().sum().sum()
SCENARIO: base | USABase_2016_new_HVDC_2

--> State
analyze
--> Loading grid
462_grid.mat not found in C:\Users\dolsen\ScenarioData\ on local machine
Transferring 462_grid.mat from server
100%|#####################################| 18.5M/18.5M [00:02<00:00, 8.13Mb/s]
Loading bus
Loading plant
Loading heat_rate_curve
Loading gencost_before
Loading gencost_after
Loading branch
Loading dcline
Loading sub
Loading bus2sub
--> Loading ct
462_ct.pkl not found in C:\Users\dolsen\ScenarioData\ on local machine
Transferring 462_ct.pkl from server
100%|#####################################| 7.25k/7.25k [00:00<00:00, 65.8kb/s]
--> Loading solar
462_solar.csv not found in C:\Users\dolsen\ScenarioData\ on local machine
Transferring 462_solar.csv from server
100%|#######################################| 104M/104M [00:14<00:00, 7.00Mb/s]
51111429.916688666

Comment thread powersimdata/input/tests/test_scale_profile.py Outdated
Comment thread powersimdata/input/scaler.py Outdated
Comment thread powersimdata/input/scaler.py
from powersimdata.input.change_table import ChangeTable
from powersimdata.input.scaler import TransformGrid

warnings.filterwarnings("ignore", category=DeprecationWarning)
Copy link
Copy Markdown
Contributor

@danielolsen danielolsen May 22, 2020

Choose a reason for hiding this comment

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

This seems a bit risky. Now that the Grid has been reverted to its classic form and we are no longer throwing DeprecationWarning, shouldn't we be willing to see DeprecationWarning errors from other packages?

Removing these does not show any additional warnings.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely. I forgot to remove these lines after the revert.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment thread powersimdata/input/tests/test_transform_grid.py Outdated
Comment thread powersimdata/scenario/analyze.py Outdated
Comment thread powersimdata/scenario/analyze.py Outdated
@danielolsen
Copy link
Copy Markdown
Contributor

Grid equality is useful for testing here:

>>> from powersimdata.scenario.scenario import Scenario
>>> scenario_544 = Scenario('544')
SCENARIO: Julia | Eastern2030Collaborative_OB2_Mesh200x3

--> State
analyze
--> Loading grid
Loading bus
Loading plant
Loading heat_rate_curve
Loading gencost_before
Loading gencost_after
Loading branch
Loading dcline
Loading sub
Loading bus2sub
--> Loading ct
>>> grid_544 = scenario_544.state.get_grid()
>>> ct_544 = scenario_544.state.get_ct()
>>> new_scenario = Scenario('')
>>> new_scenario.state.set_builder(['Eastern'])
Reading bus.csv
Reading plant.csv
Reading gencost.csv
Reading branch.csv
Reading dcline.csv
Reading sub.csv
Reading bus2sub.csv
Reading zone.csv
--> Summary
# Existing study
test | base | Julia
# Available profiles
demand: v5 | v6
hydro: v1 | v3 | v4
solar: v2 | v3.1 | v3 | v4.2 | v4
wind: v1 | v2 | v3 | v4.1 | v5.2.1 | v5.3 | v5
>>> new_scenario.state.builder.change_table.ct = ct_544
>>> new_grid = new_scenario.state.builder.get_grid()
>>> new_grid == grid_544
True

Hooray!

@danielolsen
Copy link
Copy Markdown
Contributor

What do you think about adding get_grid() and get_ct() methods to both Create and Execute? That way we can consistently call scenario.state.get_grid() and scenario.state.get_ct() and have identical behavior, no matter the state (assuming that the Create state has already run set_builder of course).

@danielolsen
Copy link
Copy Markdown
Contributor

Grid equality confirms that equality between a new grid with a change table and a grid in Execute state works too (with slightly different syntax).

>>> scenario_555 = Scenario('555')
SCENARIO: Julia | Eastern2030Independent_OB2_Mesh200x4

--> State
execute
--> Status
running
Reading bus.csv
Reading plant.csv
Reading gencost.csv
Reading branch.csv
Reading dcline.csv
Reading sub.csv
Reading bus2sub.csv
Reading zone.csv
--> Loading ct
>>> ct_555 = scenario_555.state.ct.copy()
>>> grid_555 = scenario_555.state.grid
>>> new_scenario.state.builder.change_table.ct = ct_555
>>> new_grid = new_scenario.state.builder.get_grid()
>>> new_grid == grid_555
True

@danielolsen
Copy link
Copy Markdown
Contributor

Since we can now get the ct and grid from any state (at least internally), could we also enable get_solar, get_wind and get_hydro? We could also enable get_demand(original=True).

Comment thread powersimdata/input/scaler.py
Comment thread powersimdata/input/scaler.py Outdated
:param dict ct: change table.
"""
self.grid = copy.deepcopy(grid)
self.ct = ct.copy()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure whether it matters or not, just a reminder, dict.copy() creates a shadow copy only. I think we are safe here given we just read the info from ct and never modify it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you're right, we probably want copy.deepcopy on both grid and ct. We don't want any unintentional modifications!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

A deepcopy of the change table is used when instantiating the TransformGrid and ScaleProfile classes. Also, the get_ct method in the Create, Execute and Analyze classes return a deepcopy of the change table. See last commits.

I will do an interactive rebase to clean the commit history once all your comments are addresed.

@danielolsen
Copy link
Copy Markdown
Contributor

Being able to inspect the Grid you are about to create is so, so nice. It just saved me from submitting a totally bogus offshore wind scenario.

Comment thread powersimdata/input/tests/test_scale_profile.py
Comment thread powersimdata/input/tests/test_scale_profile.py Outdated
Comment thread powersimdata/input/tests/test_transform_grid.py
Copy link
Copy Markdown
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

All my comments are resolved. All tests passed. Thanks for being patient and responsive. Great work!

@rouille rouille force-pushed the scaler_refactor branch 3 times, most recently from db034a6 to 5736700 Compare May 27, 2020 17:59
@rouille rouille force-pushed the scaler_refactor branch from 5736700 to f0453a2 Compare May 27, 2020 18:05
@rouille rouille merged commit f9454f7 into develop May 27, 2020
@rouille rouille deleted the scaler_refactor branch May 27, 2020 20:30
@ahurli ahurli mentioned this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working new feature Feature that is currently in progress. refactor Code that is being refactored

Projects

None yet

3 participants