Added support classes for transformer tap position calculation #1544
Conversation
There was a problem hiding this comment.
Thanks for all the changes done so far.
Besides the anotated points, please also have a look if there can be done reasonable changes in TransformerTappingModel (e.g. some parts might be changed from double values to squants).
Additional question: Should this PR also add things to rtd or will that happen later?
src/test/scala/edu/ie3/simona/model/control/TappingGroupModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/control/TappingGroupModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/control/TappingGroupModelSpec.scala
Outdated
Show resolved
Hide resolved
| def getTapRation: Double = tapRatio | ||
|
|
||
| /** Returns [[TransformerTappingModel.autoTap]]. | ||
| */ |
There was a problem hiding this comment.
Maybe TransomerTapping and TransformerTapping Model are better located at model.control?
There was a problem hiding this comment.
I would like to keep the transformer model with the other grid models. But we could move the ``TransformerTapping`´, since it is related to grid control.
src/main/scala/edu/ie3/simona/model/grid/TransformerTapping.scala
Outdated
Show resolved
Hide resolved
src/main/scala/edu/ie3/simona/model/grid/TransformerTapping.scala
Outdated
Show resolved
Hide resolved
src/main/scala/edu/ie3/simona/model/grid/TransformerTapping.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/control/TappingGroupModelSpec.scala
Outdated
Show resolved
Hide resolved
# Conflicts: # CHANGELOG.md
| case ranges => | ||
| // check for possible increase and decrease that can be applied to all transformers | ||
|
|
||
| // TODO: Enhance this, to support transformer combinations with different tap deltas |
There was a problem hiding this comment.
If you add the issue number to the TODO, we have later the chance to check if the TODO is solved or not.
| val deltaV = calculateVoltageDeltaFromLineCurrent( | ||
| nodeResMap, |
There was a problem hiding this comment.
Do you see a chance and sense to convert the nodeResMap to squants somewhere early? If so, one could kick the javax Dimensionless out here.
| // between both range limits => set upper limit to deltaV, lower limit is unchanged | ||
| (deltaV, deltaMinus) |
There was a problem hiding this comment.
Would this result in always tapping to the upper range, also if there is actually no congestion? No, or? Just double checking my guess.
| // greater than both range limits => limits are unchanged | ||
| (deltaPlus, deltaMinus) | ||
| case (false, false) => | ||
| // should is only possible, if deltaMinus > deltaPlus => limits are unchanged |
There was a problem hiding this comment.
Here is some grammar error / a word is missing.
| range.deltaPlus should approximate(Each(-0.03)) | ||
| range.deltaMinus should approximate(Each(-0.02)) | ||
| range.suggestion should approximate(Each(-0.02)) |
There was a problem hiding this comment.
Could you just give me some short update why these values changed (also others here)
Resolves #1543