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

Fix mesh bug existing ct #135

Merged
merged 2 commits into from
Apr 7, 2020
Merged

Fix mesh bug existing ct #135

merged 2 commits into from
Apr 7, 2020

Conversation

danielolsen
Copy link
Contributor

Purpose

Correct for a bug where calling scenario.state.builder.change_table.scale_congested_mesh_branches(scenario) will fail with a KeyError if the method is set to 'MW' or 'MWmiles' and there is an existing change table entry for a congested branch.

What is the code doing

Previously, we detected if there was an entry for a branch in ct['branch']['branch_id'], and if there was we attempted to access the entry from the top level of ct. Since there are no branch id entries in the top level of the ct, this will fail (and in fact this is how I found the bug, while trying to generate mesh scaling for Scenario 410).

In test_design_transmission.py, I add a change table where every branch has an entry and is scaled by exactly 1. This should change nothing about the results of mesh branch upgrade identification, but it triggers the aforementioned bug.

In design_transmission.py, I correct the bug so that the test passes: existing branch scaling factors for congested branches are looked up from ct['branch']['branch_id'], not from ct.

Time to review

10-15 minutes. You can check out the commit with the test, see that it fails, and then check out the tip of the fix branch and see that it fixes the problem.

@danielolsen danielolsen added the bug Something isn't working label Apr 6, 2020
@danielolsen danielolsen requested review from rouille and BainanXia April 6, 2020 18:57
@danielolsen danielolsen force-pushed the fix_mesh_bus_existing_ct branch from 858f929 to c05e625 Compare April 7, 2020 00:05
@danielolsen danielolsen force-pushed the fix_mesh_bus_existing_ct branch from c05e625 to 32ff8b4 Compare April 7, 2020 00:45
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. In the future, we might want to design some integration tests that create, run and extract and delete a dummy scenario to make sure that a new feature does not break the framework

@danielolsen
Copy link
Contributor Author

I think we could also catch similar bugs in the future by doing an automatic code coverage check when we design/run tests, to see how much the tests for a new feature cover the code for it.

@danielolsen danielolsen merged commit 03880c0 into develop Apr 7, 2020
@danielolsen danielolsen deleted the fix_mesh_bus_existing_ct branch April 7, 2020 17:46
@danielolsen danielolsen changed the title Fix mesh bus existing ct Fix mesh bug existing ct Apr 7, 2020
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants