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.

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

refactor: remove redundant function for getting bus demand #388

Merged
merged 5 commits into from
Feb 5, 2021

Conversation

danielolsen
Copy link
Contributor

@danielolsen danielolsen commented Feb 5, 2021

Purpose

Per discussion in #375, remove a redundant function. get_bus_demand is the fastest of the three identical functions:

>>> import timeit
>>> from postreise.analyze.transmission.congestion import map_demand_to_buses
>>> from powersimdata.input.input_data import get_bus_demand
>>> from powersimdata.scenario.helpers import calculate_bus_demand
>>> from powersimdata.scenario.scenario import Scenario
>>> scenario = Scenario(824)
>>> demand = scenario.state.get_demand()
>>> grid = scenario.state.get_grid()
>>> bus = grid.bus
>>> timeit.timeit(lambda: map_demand_to_buses(grid, demand), number=10)
75.39155149999999
>>> timeit.timeit(lambda: calculate_bus_demand(bus, demand), number=10)
45.56884489999993
>>> timeit.timeit(lambda: get_bus_demand(scenario.info, grid), number=10)
30.71693670000002

What the code is doing

  • The docstring and parameter names of get_bus_demand are updated to reflect reality (I think the way we called InputData changed but we didn't update all the references to it).
  • calculate_bus_demand is removed from helpers.py.
  • Analyze and Create are refactored to use get_bus_demand instead of calculate_bus_demand.
  • The construct_load_shed method of OutputData is updated to reflect the updated InputData init (can be tested before & after using scenario 398, which has old-style infeasibilities and triggers this code path).

Testing

All unit tests still pass.

Time estimate

10 minutes.

@danielolsen danielolsen self-assigned this Feb 5, 2021
@rouille
Copy link
Collaborator

rouille commented Feb 5, 2021

It looks good. Are you going to refactor PostREISE to import get_bus_demand

@danielolsen
Copy link
Contributor Author

It looks good. Are you going to refactor PostREISE to import get_bus_demand

Yes, coming up soon.

Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
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.

Looks good. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants