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

Clarified PolyPhasePolygon #3237

Merged
merged 22 commits into from
Dec 15, 2019
Merged

Clarified PolyPhasePolygon #3237

merged 22 commits into from
Dec 15, 2019

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Nov 22, 2019

I implemented a clarification about the polyphase polygon connection.
For m>3 phases there exist alternatives for the polygon,
e.g. compare m=3, m=6=2x3, m=5 and m=7:
PolyphasePolygon

@AHaumer AHaumer added enhancement New feature or enhancement P: high High priority issue documentation Issue addresses the documentation V: 3.2.3 Issue originates in MSL v3.2.3 (and is not present in earlier releases) L: Electrical.Polyphase Issue addresses Modelica.Electrical.Polyhase L: Electrical.QuasiStatic Issue addresses Modelica.Electrical.QuasiStatic labels Nov 22, 2019
@AHaumer AHaumer added this to the MSL4.0.0 milestone Nov 22, 2019
@AHaumer AHaumer self-assigned this Nov 22, 2019
Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

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

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 25, 2019

@beutlich:
Is "CI fai"l related to the fact that OM doesn't propagate the parameters of the diodes correctly?
An array of mSystems multiphase diodes, each containing mBasic singlephase diodes.

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 25, 2019

Finally resolved the type "PolyPhaseRectifier" vs. "PolyphaseRectifier".

@dietmarw
Copy link
Member

@AHaumer One thing I wondered is the naming of MultiDelta. Didn't we agree that the term "poly" is the correct one. Should then the term also be PolyDelta? Just wondering.

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 26, 2019

@dietmarw regarding "delta" vs. "polygon": You're right, polygon is the more general term, although people working on 3 phase systems are used to delta.
If we change "delta" to "polygon" in general, we have to take care of the conversion scipt, since "delta" (maybe not "multiDelta") is widely used.
Should we really make the change? @christiankral what's your opinion?
If we make the change, shouldn't we do that in a separate PR?

@dietmarw
Copy link
Member

@AHaumer You misunderstood. I did not mean the "Delta" but the "Multi". So to be consistent it should probably be "PolyDelta"?

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 26, 2019

@dietmarw
Ok I see, but the "multi" doesn't refer to the number of phases (polyphase), but to the number of (multiple) basic subsystems. Sorry to open the discussion about "delta" and "polygon" ... stop.
Just kidding: Will we end up with PolyPolgon?

@dietmarw
Copy link
Member

It's fine by me but just wanted to make sure we overlooking this and then can not change the name until the next conversion script which might or might not be a while away. You could argue that we are also talking about "multiple" phase system. Still the correct term (as we found out) is poly-phase. So just wanted to make sure that MultiDelta is the correct term.

@AHaumer AHaumer requested a review from beutlich December 8, 2019 14:00
@beutlich beutlich changed the title clarified PolyPhasePolygon Clarified PolyPhasePolygon Dec 11, 2019
@AHaumer
Copy link
Contributor Author

AHaumer commented Dec 11, 2019

@dietmarw : done (...)

Otherwise the images are not loaded correctly on a Linux system
Copy link
Contributor

@christiankral christiankral left a comment

Choose a reason for hiding this comment

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

I tested the examples with a different number of phases and everything seems to work

@AHaumer AHaumer merged commit d3a848e into modelica:master Dec 15, 2019
@AHaumer AHaumer deleted the PolyPhasePolygon branch December 30, 2019 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issue addresses the documentation enhancement New feature or enhancement L: Electrical.Polyphase Issue addresses Modelica.Electrical.Polyhase L: Electrical.QuasiStatic Issue addresses Modelica.Electrical.QuasiStatic P: high High priority issue V: 3.2.3 Issue originates in MSL v3.2.3 (and is not present in earlier releases)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants