Skip to content

Short circuits#93

Merged
Saelyos merged 16 commits intodevelopfrom
short-circuit
Jun 13, 2023
Merged

Short circuits#93
Saelyos merged 16 commits intodevelopfrom
short-circuit

Conversation

@Saelyos
Copy link
Collaborator

@Saelyos Saelyos commented Jun 8, 2023

  • Add short circuit calculation
  • Remove absolufy-imports

@Saelyos Saelyos added the enhancement New feature or request label Jun 8, 2023
@Saelyos Saelyos requested review from alihamdan and benoit9126 June 8, 2023 13:15
@Saelyos Saelyos self-assigned this Jun 8, 2023
Copy link
Member

@benoit9126 benoit9126 left a comment

Choose a reason for hiding this comment

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

Great for pre-commit. I will propagate this configuration to our other libraries.

I would also add some features in this PR:

  • The possibility to remove a short-circuit (or maybe to clear them)
  • The ability to have a data frame of shorts circuits in the ElectricalNetwork instance
  • How do you handle short-circuits between the three phases?
  • [Disclaimer: potential Newby question] Why is the neutral excluded?

@Saelyos
Copy link
Collaborator Author

Saelyos commented Jun 8, 2023

@benoit9126

  • Removing a short-circuit is not something we can have in the engine part, but we can do it here if we want (it will be overridden in engine to raise an exception).
  • I'll add that
  • I've just added it (we have discussed about it with @alihamdan for the interface)
  • I'm not sure what you're referring to, I've added a test with only "abc" phases, but otherwise "n" is allowed

Copy link
Member

@alihamdan alihamdan left a comment

Choose a reason for hiding this comment

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

Left you some comments

(
bus.id,
bus.phases,
"".join(bus._short_circuit),
Copy link
Member

Choose a reason for hiding this comment

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

I would have added a sorted in order to have "abcn" rather than "nbca": "".join(sorted(bus._short_circuit))

@benoit9126
Copy link
Member

@Saelyos Great! Forget what I wrote about the neutral. I read your code too quickly

@benoit9126
Copy link
Member

@Saelyos Do not forget to add documentation on short circuits.

@benoit9126
Copy link
Member

@Saelyos A few additional questions/remarks:

  • Could you please add some examples showing that the library protects the user from inconsistent behaviour (in the notebook) ? Example: adding a PowerLoad in a bus with a short circuit, etc.
  • Currently, it is impossible to model two short circuits in the same bus. Example: I have a ABCN bus and I would like to model a short circuit between the phase A and B and another between the phases C and N. ⇾ It should not be common to do such things, I suppose.

@Saelyos Saelyos merged commit b8c8994 into develop Jun 13, 2023
@Saelyos Saelyos deleted the short-circuit branch June 13, 2023 12:22
@benoit9126 benoit9126 mentioned this pull request Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants