tf cos phi power factor#3064
Conversation
…07-TF-cos-phi-power-factor
…07-TF-cos-phi-power-factor
|
Provide functionality to write "cos phi" columns to load and sgen and then sync them to p. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3064 +/- ##
===========================================
+ Coverage 71.67% 71.69% +0.01%
===========================================
Files 355 355
Lines 39348 39381 +33
===========================================
+ Hits 28204 28235 +31
- Misses 11144 11146 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
KS-HTK
changed the title
assert via allclose, replace .values with to_numpy, added changelog entry
|
Thank you for the review! |
| return _pq_from_cosphi_bulk(s, cosphi, qmode, pmode, len_=len_) | ||
|
|
||
|
|
||
| def create_cos_phi_from_network(net: pandapowerNet, element_type: str) -> None: |
There was a problem hiding this comment.
I would rename this function, create_cos_phi(net: pandapowerNet, element_type: Literal["sgen", "gen", "load"])
There was a problem hiding this comment.
create_cos_phi_from_network_state?
Note that gen (PV generator) makes no sense
There was a problem hiding this comment.
No I would remove the "from_network*" since the first parameter is the network and nothing else. So it is kinda clear, that it is from / for the network.
PV gen we thought about, but this needs to be handled differently, I guess.
| table["cos_phi"] = cos_phi | ||
|
|
||
|
|
||
| def create_cos_phi_constant( |
There was a problem hiding this comment.
This I would rename to set_cos_phi(net: pandapowerNet, element_type: Literal["load", "sgen", "gen"], cos_phi: float = 0.95, mode: Literal["underexcited", "overexcited"] = "underexcited")
There was a problem hiding this comment.
set_constant_cos_phi? again, gen makes no sense
There was a problem hiding this comment.
it is not really constant, it is just a cos_phi value. Again nowhere in pp it is called like that. Also setting this value, does not enforce that it is constant (no logic is available in pp for doing that), in essence it could just be ignored by other parts of the code base.
| table["cos_phi"] = sign * abs(cos_phi) | ||
|
|
||
|
|
||
| def sync_q_from_cos_phi(net: pandapowerNet, element_type: str, indices: Any) -> None: |
There was a problem hiding this comment.
this I would change to update_via_cos_phi(net: pandapowerNet, element_type: str, indices: Iterable[Int] | Int, direction: Literal["q_from_p", "p_from_q"] = "q_from_p")
Please implement both directions.
There was a problem hiding this comment.
sync p from q makes no sense.
There was a problem hiding this comment.
That is the part I do not know, maybe in some kind of optimization it could be interesting. But If you think it is unnecessary, just omit it.
…wer-factor' into feature/2026-01-07-TF-cos-phi-power-factor
|
| table["cos_phi"] = cos_phi | ||
|
|
||
|
|
||
| def set_constant_cos_phi( |
There was a problem hiding this comment.
A constant does not need to be set, either it is constant, then it should be declared as such (see pep8 on constants), or it is not constant and should not be named as such.
3 participants
Uh oh!
There was an error while loading. Please reload this page.