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

Make bus element checks in create_switches optional #2343

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

thomas-whaley-poco
Copy link

@thomas-whaley-poco thomas-whaley-poco commented Jul 15, 2024

This PR makes the bus element checks in create_switches optional as specified in #2303 .

I am writing a pipeline that converts large DataFrames to a large pandapower network. I estimate the size of the DataFrame containing switch data to be over 50,000 rows. Simply running the create_switches function with this DataFrame takes 10-20 minutes, whereas functions like create_lines take less than a minute with a similar sized DataFrame.

I understand this is caused because it is checking if the bus and element matches the element type and is a valid bus. In my situation, I would prefer to have this function run in less than a minute than being sure the busses are valid.

This PR introduces a new flag in create_switches (validate_busses=True), which would determine if the validation loop runs or not.

@thomas-whaley-poco thomas-whaley-poco changed the title Make bus element checks in create_switches optional #2303 Make bus element checks in create_switches optional Jul 15, 2024
@SteffenMeinecke
Copy link
Member

I think, we can add a flag to enable not checking that switches are added only to valid buses and elements (should not be named validate_busses because elements are checked for validity, too (and buses is without double s)).
Nevertheless, we should add a CAUTION to the docstring explanation.
@thomas-whaley-poco you have an example where 50,000 switches should be created. Can you use that example to accelerate the validation code by vectorizing the code (replace the for loops)?

@thomas-whaley-poco
Copy link
Author

@SteffenMeinecke just vectorized the bus/element validation. Now takes less than a second to run 50,000 switches.

@thomas-whaley-poco
Copy link
Author

The tests in pandapower/test/protection/test_oc_relay_model.py are failing now because of pandapower/protection/example_grids::dtoc_relay, but they seem suspicious to begin with, as the line switches seem to not be correctly connected (buses are equal to the elements)? @SteffenMeinecke please correct me if I am wrong!

@SteffenMeinecke
Copy link
Member

SteffenMeinecke commented Aug 8, 2024

I agree: The switches are not correctly defined in https://github.com/e2nIEE/pandapower/blob/develop/pandapower/protection/example_grids.py#L27 and https://github.com/e2nIEE/pandapower/blob/develop/pandapower/protection/example_grids.py#L54 and https://github.com/e2nIEE/pandapower/blob/develop/pandapower/protection/example_grids.py#L88 and maybe also at all other positions of the file where switches are created.
@arjunmadhusoodhanan and @gourab2009 can you please fix that?

@SteffenMeinecke
Copy link
Member

After talking to @gourab2009, it seems that I did not checked sufficiently. @thomas-whaley-poco, isn't it possible that the switches are created correctly in the tests?
In https://github.com/e2nIEE/pandapower/blob/develop/pandapower/protection/example_grids.py#L27 for example, the switches are defined between the (from) buses (0, 1, .., 5, 2, 7, ...) and the lines (0, 1, ...) which is ok.
Are this PR's changes the reason for the failing tests?

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.40%. Comparing base (57c60a1) to head (b154ccd).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2343      +/-   ##
===========================================
- Coverage    76.40%   76.40%   -0.01%     
===========================================
  Files          280      280              
  Lines        32427    32420       -7     
===========================================
- Hits         24776    24770       -6     
+ Misses        7651     7650       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@thomas-whaley-poco
Copy link
Author

@SteffenMeinecke Apologies for the delay, yes that is true, the original code causes the tests to pass. I'll resolve this.

@thomas-whaley-poco
Copy link
Author

@SteffenMeinecke after digging around in the old code, I've found that the new code in the PR has the exact same functionality as the old code, except for this line: https://github.com/e2nIEE/pandapower/blob/develop/pandapower/create.py#L4446.
The old code was zipping unexpectedly when et was a single string. The iterator created by zip(elements, et, buses) would be exhausted after one iteration because et = 'l'.

This meant that while the tests were passing, it actually wasn't testing the entire contents of the input given into this function, hence my new code failing.

If I replace zip(elements, et, buses) with zip(elements, [et] * len(buses), buses), it causes the tests to fail where my tests failed.

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