Skip to content

Commit

Permalink
adapt pr to do sanity check for multiple osmEntities
Browse files Browse the repository at this point in the history
  • Loading branch information
danielfeismann committed May 17, 2024
1 parent e9c9c18 commit fdc009a
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 115 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Switched from `akka` to `pekko`
- Adding the clustering of low voltage grids
- Consider substations as type `Node` [#411](https://github.com/ie3-institute/OSMoGrid/issues/411)
- Added sanity check for multiple OsmEntities [#422](https://github.com/ie3-institute/OSMoGrid/issues/422)

### Changed
- Rely on Java 17
Expand All @@ -41,7 +42,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Switched from `osm4scala` to `openstreetmap.osmosis` [#409](https://github.com/ie3-institute/OSMoGrid/issues/409)
- Changed transformer input parameter to PSDM requirements [#417](https://github.com/ie3-institute/OSMoGrid/issues/417)
- Adapted run initialization [#404](https://github.com/ie3-institute/OSMoGrid/issues/404)
- Only consider newest version of `Ways` [#422](https://github.com/ie3-institute/OSMoGrid/issues/422)

### Fixed
- Fixed bug in `LvGridGeneratorSupport` [#388](https://github.com/ie3-institute/OSMoGrid/issues/388)
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/edu/ie3/osmogrid/io/input/ReaderSink.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

package edu.ie3.osmogrid.io.input

import edu.ie3.osmogrid.exception.PbfReadFailedException
import edu.ie3.osmogrid.exception.{InputDataException, PbfReadFailedException}
import edu.ie3.osmogrid.model.OsmoGridModel.LvOsmoGridModel
import edu.ie3.osmogrid.model.{OsmoGridModel, SourceFilter}
import edu.ie3.util.osm.model.OsmContainer.ParOsmContainer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@ import edu.ie3.datamodel.models.input.connector.`type`.{
}
import edu.ie3.datamodel.models.input.container.SubGridContainer
import edu.ie3.datamodel.models.input.system.LoadInput
import edu.ie3.datamodel.models.voltagelevels.VoltageLevel
import edu.ie3.datamodel.models.voltagelevels.{
GermanVoltageLevelUtils,
VoltageLevel
}
import edu.ie3.osmogrid.exception.IllegalStateException
import edu.ie3.osmogrid.graph.OsmGraph
import edu.ie3.osmogrid.lv.LvGraphGeneratorSupport.BuildingGraphConnection
import edu.ie3.util.osm.model.OsmEntity.Node
import tech.units.indriya.ComparableQuantity
import utils.Clustering
import utils.Clustering.Cluster
import utils.GridConversion._

import javax.measure.quantity.ElectricPotential
import scala.annotation.tailrec
import scala.collection.Set
import scala.collection.parallel.{ParMap, ParSeq}
Expand Down
62 changes: 12 additions & 50 deletions src/main/scala/edu/ie3/osmogrid/model/OsmoGridModel.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

package edu.ie3.osmogrid.model

import com.typesafe.scalalogging.LazyLogging
import edu.ie3.osmogrid.model.SourceFilter.{Filter, LvFilter}
import edu.ie3.util.osm.model.OsmContainer.ParOsmContainer
import edu.ie3.util.osm.model.OsmEntity.Relation.RelationMemberType
Expand All @@ -24,7 +23,7 @@ sealed trait OsmoGridModel {
def +(additional: OsmoGridModel): Option[OsmoGridModel]
}

object OsmoGridModel extends LazyLogging {
object OsmoGridModel {

def filterForSubstations(
entities: ParSeq[EnhancedOsmEntity]
Expand Down Expand Up @@ -57,13 +56,14 @@ object OsmoGridModel extends LazyLogging {
}
(matchedEntities.par, matchedSubentities)
}

def filterForWays(
entities: ParSeq[EnhancedOsmEntity]
): (ParSeq[Way], Map[Long, Node]) = {
val (matchedEntities, matchedSubentities) =
filterForOsmType[Way, Node](entities)
// sanity check if there multiple editions of single osm entities
sanityCheckForMultipleOsmEntities(entities)
val (latestEntities, latestNodes) =
filterForLatestVersions(matchedEntities, matchedSubentities)
filterForOsmType[Way, Node](entities)

val ways = latestEntities.collect { case way: Way => way }
(ways, latestNodes)
Expand All @@ -72,10 +72,8 @@ object OsmoGridModel extends LazyLogging {
def filterForClosedWays(
entities: ParSeq[EnhancedOsmEntity]
): (ParSeq[ClosedWay], Map[Long, Node]) = {
val (matchedEntities, matchedSubentities) =
filterForOsmType[ClosedWay, Node](entities)
val (latestEntities, latestNodes) =
filterForLatestVersions(matchedEntities, matchedSubentities)
filterForOsmType[ClosedWay, Node](entities)

val closedWays = latestEntities.collect { case way: ClosedWay => way }
(closedWays, latestNodes)
Expand Down Expand Up @@ -107,53 +105,17 @@ object OsmoGridModel extends LazyLogging {
(matchedEntities.par, matchedSubentities)
}

def filterForLatestVersions[
E <: OsmEntity: ClassTag,
S <: OsmEntity: ClassTag
](entities: ParSeq[E], nodes: Map[Long, S]): (ParSeq[E], Map[Long, S]) = {
def sanityCheckForMultipleOsmEntities(entities: ParSeq[EnhancedOsmEntity]) = {

val groupedEntities = entities.groupBy(_.id)
val groupedEntities = entities.groupBy(_.entity.id)

val (multipleOccurrenceGroups, singleOccurrenceGroups) =
groupedEntities.partition { case (_, group) => group.size > 1 }

val latestEntitiesFromMultipleOccurrences = multipleOccurrenceGroups.values
.flatMap { entities =>
val maxVersion = entities
.flatMap {
case entity: E if entity.metaInformation.isDefined =>
entity.metaInformation.map(_.version.getOrElse(0))
case _ => Some(0)
}
.reduceOption(_ max _)
.getOrElse(0)
entities.filter {
case entity: E if entity.metaInformation.isDefined =>
entity.metaInformation.exists(_.version.contains(maxVersion))
case entity: E if !entity.metaInformation.isDefined =>
logger.info(
s"Warning: ${entity.id} does not contain any meta information to identify the version id of the OsmEntity, so it was filtered out."
)
false
}
}
.toSeq
.par

val singleOccurrenceEntities = singleOccurrenceGroups.values.flatten.toSeq

// Combine both sets of entities
val combinedEntities =
(latestEntitiesFromMultipleOccurrences ++ singleOccurrenceEntities).par

val nodeIds = combinedEntities.flatMap {
case entity: Way => entity.nodes
case _ => Seq.empty
}.toSet

val latestNodes = nodes.view.filterKeys(nodeIds).toMap

(combinedEntities, latestNodes)
if (multipleOccurrenceGroups.nonEmpty)
throw new RuntimeException(
s"Osm data does contain OsmEntity that occur multiple times with the same id: ${multipleOccurrenceGroups.values}"
)
}

final case class LvOsmoGridModel(
Expand Down
27 changes: 8 additions & 19 deletions src/test/scala/edu/ie3/osmogrid/model/OsmTestData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ trait OsmTestData {
val highway2Node1: Node = highway1Node2
val highway2Node2: Node =
Node(23L, 50.538d, 7.4065d, Map.empty[String, String], None)
val highway2Node2b: Node =
Node(123L, 50.5382d, 7.40652d, Map.empty[String, String], None)
val highway2Node3: Node =
Node(24L, 50.578d, 7.4265d, Map.empty[String, String], None)
val highway2Node4: Node = highway1Node1
Expand Down Expand Up @@ -152,7 +150,7 @@ trait OsmTestData {
nodes.building1Node1.id
),
Map("building" -> "yes"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val building2: ClosedWay =
Expand All @@ -166,7 +164,7 @@ trait OsmTestData {
nodes.building2Node1.id
),
Map("building" -> "yes"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val building3: ClosedWay =
Expand All @@ -180,15 +178,15 @@ trait OsmTestData {
nodes.building3Node1.id
),
Map("building" -> "yes"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val highway1: OpenWay =
OpenWay(
111L,
Seq(nodes.highway1Node1.id, nodes.highway1Node2.id),
Map("highway" -> "motorway"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val highway2: OpenWay =
Expand All @@ -201,7 +199,7 @@ trait OsmTestData {
nodes.highway2Node4.id
),
Map("highway" -> "motorway"),
Some(OsmEntity.MetaInformation(Some(1), None, None, None, None, None))
None
)

val highway2oldVersion: OpenWay =
Expand All @@ -210,19 +208,10 @@ trait OsmTestData {
Seq(
nodes.highway2Node1.id,
nodes.highway2Node2.id,
nodes.highway2Node2b.id,
nodes.highway2Node3.id,
nodes.highway2Node4.id
),
Map("highway" -> "motorway"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
)

val highway4: OpenWay =
OpenWay(
114L,
Seq(nodes.highway1Node1.id, nodes.highway1Node2.id),
Map("highway" -> "motorway"),
None
)

Expand All @@ -236,7 +225,7 @@ trait OsmTestData {
nodes.landuse1Node1.id
),
Map("landuse" -> "education"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val landuse2: ClosedWay = ClosedWay(
Expand All @@ -249,7 +238,7 @@ trait OsmTestData {
nodes.landuse2Node1.id
),
Map.empty[String, String],
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val landuse3: ClosedWay = ClosedWay(
Expand All @@ -262,7 +251,7 @@ trait OsmTestData {
nodes.landuse3Node1.id
),
Map("landuse" -> "residential"),
Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None))
None
)

val boundaryWay1: OpenWay = OpenWay(
Expand Down
55 changes: 14 additions & 41 deletions src/test/scala/edu/ie3/osmogrid/model/OsmoGridModelSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import edu.ie3.osmogrid.model.SourceFilter.{Filter, LvFilter}
import edu.ie3.test.common.UnitSpec
import edu.ie3.util.osm.model.OsmContainer.ParOsmContainer
import edu.ie3.util.osm.model.OsmEntity.Relation
import edu.ie3.util.osm.model.OsmEntity.Way.{ClosedWay, OpenWay}

import scala.collection.parallel.CollectionConverters._
import scala.collection.parallel.ParSeq

Expand Down Expand Up @@ -406,7 +404,7 @@ class OsmoGridModelSpec extends UnitSpec with OsmTestData {

}

"filter for correct version of way" in {
"throw exception for OsmEntities that occur multiple times" in {
val relation = EnhancedOsmEntity(
relations.boundary,
Map(nodes.boundaryNode1.id -> nodes.boundaryNode1)
Expand All @@ -431,7 +429,6 @@ class OsmoGridModelSpec extends UnitSpec with OsmTestData {
Map(
nodes.highway2Node1.id -> nodes.highway2Node1,
nodes.highway2Node2.id -> nodes.highway2Node2,
nodes.highway2Node2b.id -> nodes.highway2Node2b,
nodes.highway2Node3.id -> nodes.highway2Node3,
nodes.highway2Node4.id -> nodes.highway2Node4
)
Expand All @@ -444,45 +441,21 @@ class OsmoGridModelSpec extends UnitSpec with OsmTestData {
highway2oldVersion
)

val (resultWays, resultWayNodes) =
intercept[RuntimeException] {
OsmoGridModel.filterForWays(input)
}.getMessage shouldBe "Osm data does contain OsmEntity that occur multiple times with the same id: " +
"ParIterable(ParArray(" +
"EnhancedOsmEntity(OpenWay(112,List(22, 23, 24, 21),Map(highway -> motorway),None)," +
"Map(22 -> Node(22,51.498,7.4255,Map(),None), " +
"23 -> Node(23,50.538,7.4065,Map(),None), " +
"24 -> Node(24,50.578,7.4265,Map(),None), " +
"21 -> Node(21,51.4955,7.4063,Map(),None))), " +
"EnhancedOsmEntity(OpenWay(112,List(22, 23, 24, 21),Map(highway -> motorway),None)," +
"Map(22 -> Node(22,51.498,7.4255,Map(),None), " +
"23 -> Node(23,50.538,7.4065,Map(),None), " +
"24 -> Node(24,50.578,7.4265,Map(),None), " +
"21 -> Node(21,51.4955,7.4063,Map(),None)))))"

resultWays.size shouldBe 2

val closedWays = resultWays.collect { case way: ClosedWay => way }
val openWays = resultWays.collect { case way: OpenWay => way }

closedWays.size shouldBe 1
openWays.size shouldBe 1

closedWays.headOption.getOrElse(
fail("No closed way found")
) shouldBe ways.building1
openWays.headOption.getOrElse(
fail("No open way found")
) shouldBe ways.highway2

resultWayNodes.size shouldBe 5
resultWayNodes.getOrElse(
nodes.building1Node1.id,
fail("Collection shouldn't be empty")
) shouldBe nodes.building1Node1

val expectedNodes = List(
nodes.highway2Node1,
nodes.highway2Node2,
nodes.highway2Node3,
nodes.highway2Node4,
nodes.building1Node1
)

expectedNodes.foreach { node =>
resultWayNodes.getOrElse(node.id, fail("Node not found")) shouldBe node
}

resultWayNodes.foreach { case (_, node) =>
expectedNodes should contain(node)
}
}
}
}
3 changes: 1 addition & 2 deletions src/test/scala/edu/ie3/test/common/OsmTestData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

package edu.ie3.test.common

import edu.ie3.util.osm.model.OsmEntity
import edu.ie3.util.osm.model.OsmEntity.{Node, Way}

trait OsmTestData {
Expand Down Expand Up @@ -42,7 +41,7 @@ trait OsmTestData {
id,
Seq(nodeA.id, nodeB.id),
Map.empty,
Some(OsmEntity.MetaInformation(Some(0)))
None
)
}
}

0 comments on commit fdc009a

Please sign in to comment.