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 power connectors again #419

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Conversation

pierre-eliep-met
Copy link
Collaborator

@pierre-eliep-met pierre-eliep-met commented May 9, 2023

Goal

Fixes #99 again, update of #278.

What was the problem ?

Power connectors were not balanced because :

  • in modelica connectors must always have the same number of flow and potential variables.
  • we had only one flow variable in power connectors : W

Solution

  • Add a dummy potential variable in connectors
  • Set this new variable only once in a power connection set (since potential are equal in every node of the connection set, just like pressure)

Implementation

  • A dummy variable is created in power inlet and outlet connectors
  • Then, the dummy variable is set in generator inlet power connector (because we always have only 1 generator in a connection set), and same in power sensor inlet and sink inlet
  • In all other components with power connectors, a few modifications are made :
    • Gas turbine, compressor : dummy variable of power connectors is set as a connector input
    • pump : no more power connector in pumps, no more need to put a power source along with pumps
    • Steam generator : SGs now have a power connector, and we need to define the dummy of this connector, so I just added a power sink in the SG, and since dummy=0 is defined in the sink, it defines it into the connection set there.

Warnings

Note that this is a bit weird and not super flexible/robust because you could be willing to connect some power components that would both try to set the dummy variable, resulting in an over constrained model.
But it shouldn't be a problem (as of now), because we should never face this issue, because we would probably never have a single connection set with several generators/power sensors/power sinks, etc.

Type of change

  • Bugfix
  • New feature
  • Refactoring change
  • Release & Version Update (don't forget to change the version number in package.mo)

Will it break anything in previous models ?

  • Breaking change (If yes, make sure to point it out in the changelog)
  • Non-Breaking change

Checklist

  • I have added the appropriate tags, reviewers, projects (and detailed the size and priority of my PR) and linked issues to this PR
  • I have performed a self-review of my own code
  • I have checked that all existing tests pass.
  • I have added/updated tests that prove my development works and does not break anything.
  • I have made corresponding changes or additions to the documentation (in Notion documentation)
  • I have added corresponding entries to the Changelog
  • I have checked for conflicts with target branch, and merged/rebased in consequence

You can also fill these out after creating the PR, but make sure to check them all before submitting your PR for review.

@pierre-eliep-met pierre-eliep-met marked this pull request as ready for review May 9, 2023 12:52
@pierre-eliep-met pierre-eliep-met self-assigned this May 9, 2023
@pierre-eliep-met pierre-eliep-met added the ⚖️ Local Balance Component is not locally balanced label May 9, 2023
@pierre-eliep-met
Copy link
Collaborator Author

I've just spotted a problem on compressor unit tests 😭

@valentind-met
Copy link
Collaborator

Seems like it's working. However, it seems like a lot of work to be able to implement multi nodes with power connectors. It brings added complexity in understanding the library, with a lot of unused variables...
I am now thinking it would be a more reasonnable solution to use RealConnectors and a "sum" component.

@pierre-eliep-met pierre-eliep-met marked this pull request as draft May 11, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚖️ Local Balance Component is not locally balanced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Power connectors are not locally balanced because they only contain one flow variable : W
2 participants