Bump com.github.ie3-institute:PowerSystemDataModel from 7.0.0 to 8.1.0#599
Conversation
dependabot
bot
added
dependencies
dependabot
bot
requested review from
danielfeismann,
sebastian-peter and
staudtMarius
as code owners
Bumps [com.github.ie3-institute:PowerSystemDataModel](https://github.com/ie3-institute/PowerSystemDatamodel) from 7.0.0 to 8.1.0. - [Release notes](https://github.com/ie3-institute/PowerSystemDatamodel/releases) - [Changelog](https://github.com/ie3-institute/PowerSystemDataModel/blob/dev/CHANGELOG.md) - [Commits](ie3-institute/PowerSystemDataModel@7.0.0...8.1.0) --- updated-dependencies: - dependency-name: com.github.ie3-institute:PowerSystemDataModel dependency-version: 8.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
bot
force-pushed
the
dependabot/gradle/dev/com.github.ie3-institute-PowerSystemDataModel-8.1.0
branch
from
f66134c to
34ba3f8
Compare
…e-PowerSystemDataModel-8.1.0
staudtMarius
left a comment
staudtMarius
left a comment
There was a problem hiding this comment.
Here are some comments from my side.
src/main/scala/edu/ie3/osmogrid/guardian/run/SubGridHandling.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/osmogrid/guardian/run/SubGridHandlingSpec.scala
Outdated
Show resolved
Hide resolved
…e-PowerSystemDataModel-8.1.0
…e-PowerSystemDataModel-8.1.0
sebastian-peter
left a comment
sebastian-peter
left a comment
There was a problem hiding this comment.
Some questions and comments from my side :)
src/test/scala/edu/ie3/osmogrid/guardian/run/SubGridHandlingSpec.scala
Outdated
Show resolved
Hide resolved
src/main/scala/edu/ie3/osmogrid/guardian/run/SubGridHandling.scala
Outdated
Show resolved
Hide resolved
| "handle all received grid results" in new GridSupport { | ||
| val lvGrids: Seq[SubGridContainer] = Seq(mockSubGrid(1)) | ||
| val mvGrids: Seq[SubGridContainer] = Seq(mockSubGrid(3)) | ||
| val streetGraph: OsmGraph = new OsmGraph() | ||
|
|
||
| /* Result is forwarded to listener */ | ||
| // LV first | ||
| runningTestKit.run( | ||
| MessageAdapters.WrappedLvCoordinatorResponse( | ||
| RepLvGrids(lvGrids, streetGraph) | ||
| ) | ||
| ) | ||
|
|
||
| // MV | ||
| runningTestKit.run( | ||
| MessageAdapters.WrappedMvCoordinatorResponse( | ||
| RepMvGrids(mvGrids, None, Map.empty, assetInformation) | ||
| ) | ||
| ) | ||
|
|
||
| runningTestKit.run(HandleGridResults) | ||
| resultListener.expectMessageType[GridResult] |
There was a problem hiding this comment.
Wondering how this worked before, and why now these additional messages are necessary. Maybe you have more insight?
There was a problem hiding this comment.
Thats a very good question! I just remember that it broke and it looked like the GridResults were not there and thus couldn't be handled.
There was a problem hiding this comment.
I debugged through the former test code, and it threw the following:
edu.ie3.datamodel.exceptions.InvalidGridException: Cannot determine the predominant voltage level. Following voltage levels are present:
So I think something in this test is not properly working...
There was a problem hiding this comment.
I had some deep look on this. With #1366 we updated ContainerUtils.determinePredominantVoltLvl() and this now filters the nodes of rawGrid by the subsetNo.
Map<VoltageLevel, Long> voltageLevelCount =
rawGrid.getNodes().stream()
.filter(n -> n.getSubnet() == subnet)
This leads to the situation that the inital subset number of our mocked mv grid is 100 but get later updated in SubGridHandling.updateNodeSubNetNumbers() to be 3. However, this node will than get filtered out, which lead to the case that no predominant voltage level can be determined since this is the only node in the grid.
Solution: mock subgrid initally with subsetNo = 3.
There was a problem hiding this comment.
Sorry, I think there's still a deeper problem here. What you've done now is that you gave the MV grid the subgrid number that it would be assigned later in SubGridHandling.updateNodeSubNetNumbers. However, we cannot expect the MvCoordinator, who orders the MV grid creation, to assign exactly the number that we would re-assign in updateNodeSubNetNumbers later.
(If you're curious, the MV grid coordinator picks the sub grid number here:)
OSMoGrid/src/main/scala/edu/ie3/osmogrid/mv/MvCoordinator.scala
Lines 264 to 270 in aacc4e7
I've dug a little deeper, and this is where the exception occurs (this is what you described, Daniel, but this also for me so that I'll remember later... It's always a fight to dig into the rabbit hole again):
https://github.com/ie3-institute/PowerSystemDataModel/blob/91d5c3eb591cae4f32d912886aeb54f78d37fcab/src/main/java/edu/ie3/datamodel/models/input/container/SubGridContainer.java#L23-L33
determinePredominantVoltLvl crashes here because we are supplying it with a grid that has already adapted its node (which now has subgrid number 3), but the int subnet we're supplying is still 100. So in my opinion, this would also happen in a similar fashion if you run OSMoGrid normally.
Why was this not an issue before? The earlier imlementation of determinePredominantVoltLvl (before it changed with ie3-institute/PowerSystemDataModel#1366) did not really use the supplied subnet number to filter for the nodes of the actual sub grid. Now it does (which is fine imho), but this means we have to make sure that the subgrid number in SubGridContainer is actually the correct one.
But, I think if we update SubGridHandling.processResults to also update the subgrid numbers of the SubGridContainers, we're fine.
There was a problem hiding this comment.
Why was this not an issue before? The earlier imlementation of determinePredominantVoltLvl (before it changed with ie3-institute/PowerSystemDataModel#1366) did not really use the supplied subnet number to filter for the nodes of the actual sub grid. Now it does (which is fine imho), but this means we have to make sure that the subgrid number in SubGridContainer is actually the correct one.
Yes! That is also what I realized last week. I agree, the filter is right there and thus we should solve this here. Thanks that you took the time to dig that deep into this.
There was a problem hiding this comment.
It might be caused some where here
OSMoGrid/src/main/scala/utils/GridConversion.scala
Lines 263 to 265 in aacc4e7
We implicitly provide 1 as subnetnumber to each lv subgrid (and also the same uuid), but filter later e.g. for subnet 2.
Co-authored-by: Sebastian Peter <sebastian.peter@tu-dortmund.de>
…e-PowerSystemDataModel-8.1.0
…e-PowerSystemDataModel-8.1.0
…e-PowerSystemDataModel-8.1.0
3 participants
Bumps com.github.ie3-institute:PowerSystemDataModel from 7.0.0 to 8.1.0.
Release notes
Sourced from com.github.ie3-institute:PowerSystemDataModel's releases.
Changelog
Sourced from com.github.ie3-institute:PowerSystemDataModel's changelog.
Commits
21ed55dMerge pull request #1399 from ie3-institute/rel/df/#1398-release_8.1.04dcd202update citation cff90a7fd1update changelog690f348changelog4a1adb6Merge branch 'dev' into rel/df/#1398-release_8.1.0fd1758cMerge pull request #1358 from ie3-institute/df/#1357-validation-load835e05ffix test7da594afmt and docs085cad5Merge remote-tracking branch 'origin/df/#1357-validation-load' into df/#1357-...9095892refactor supported load profilesYou can trigger a rebase of this PR by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)