From fdc009a7c60aa22b804901fc60c5549166df05db Mon Sep 17 00:00:00 2001 From: danielfeismann Date: Fri, 17 May 2024 08:19:51 +0200 Subject: [PATCH] adapt pr to do sanity check for multiple osmEntities --- CHANGELOG.md | 2 +- .../ie3/osmogrid/io/input/ReaderSink.scala | 2 +- .../osmogrid/lv/LvGridGeneratorSupport.scala | 7 ++- .../ie3/osmogrid/model/OsmoGridModel.scala | 62 ++++--------------- .../edu/ie3/osmogrid/model/OsmTestData.scala | 27 +++----- .../osmogrid/model/OsmoGridModelSpec.scala | 55 +++++----------- .../edu/ie3/test/common/OsmTestData.scala | 3 +- 7 files changed, 43 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 642e9e84..b116b64c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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) diff --git a/src/main/scala/edu/ie3/osmogrid/io/input/ReaderSink.scala b/src/main/scala/edu/ie3/osmogrid/io/input/ReaderSink.scala index 7a5830a9..fe1536f2 100644 --- a/src/main/scala/edu/ie3/osmogrid/io/input/ReaderSink.scala +++ b/src/main/scala/edu/ie3/osmogrid/io/input/ReaderSink.scala @@ -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 diff --git a/src/main/scala/edu/ie3/osmogrid/lv/LvGridGeneratorSupport.scala b/src/main/scala/edu/ie3/osmogrid/lv/LvGridGeneratorSupport.scala index 8320ed3c..eb2cffed 100644 --- a/src/main/scala/edu/ie3/osmogrid/lv/LvGridGeneratorSupport.scala +++ b/src/main/scala/edu/ie3/osmogrid/lv/LvGridGeneratorSupport.scala @@ -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} diff --git a/src/main/scala/edu/ie3/osmogrid/model/OsmoGridModel.scala b/src/main/scala/edu/ie3/osmogrid/model/OsmoGridModel.scala index 09d6d4ac..3fbe21c8 100644 --- a/src/main/scala/edu/ie3/osmogrid/model/OsmoGridModel.scala +++ b/src/main/scala/edu/ie3/osmogrid/model/OsmoGridModel.scala @@ -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 @@ -24,7 +23,7 @@ sealed trait OsmoGridModel { def +(additional: OsmoGridModel): Option[OsmoGridModel] } -object OsmoGridModel extends LazyLogging { +object OsmoGridModel { def filterForSubstations( entities: ParSeq[EnhancedOsmEntity] @@ -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) @@ -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) @@ -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( diff --git a/src/test/scala/edu/ie3/osmogrid/model/OsmTestData.scala b/src/test/scala/edu/ie3/osmogrid/model/OsmTestData.scala index 439c8f33..b10e8c4b 100644 --- a/src/test/scala/edu/ie3/osmogrid/model/OsmTestData.scala +++ b/src/test/scala/edu/ie3/osmogrid/model/OsmTestData.scala @@ -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 @@ -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 = @@ -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 = @@ -180,7 +178,7 @@ trait OsmTestData { nodes.building3Node1.id ), Map("building" -> "yes"), - Some(OsmEntity.MetaInformation(Some(0), None, None, None, None, None)) + None ) val highway1: OpenWay = @@ -188,7 +186,7 @@ trait OsmTestData { 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 = @@ -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 = @@ -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 ) @@ -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( @@ -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( @@ -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( diff --git a/src/test/scala/edu/ie3/osmogrid/model/OsmoGridModelSpec.scala b/src/test/scala/edu/ie3/osmogrid/model/OsmoGridModelSpec.scala index b06e1f78..386c80ae 100644 --- a/src/test/scala/edu/ie3/osmogrid/model/OsmoGridModelSpec.scala +++ b/src/test/scala/edu/ie3/osmogrid/model/OsmoGridModelSpec.scala @@ -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 @@ -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) @@ -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 ) @@ -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) - } } } } diff --git a/src/test/scala/edu/ie3/test/common/OsmTestData.scala b/src/test/scala/edu/ie3/test/common/OsmTestData.scala index b0214b17..8287240e 100644 --- a/src/test/scala/edu/ie3/test/common/OsmTestData.scala +++ b/src/test/scala/edu/ie3/test/common/OsmTestData.scala @@ -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 { @@ -42,7 +41,7 @@ trait OsmTestData { id, Seq(nodeA.id, nodeB.id), Map.empty, - Some(OsmEntity.MetaInformation(Some(0))) + None ) } }