From a88daf9a2fe98167c666e99c872c3172281fb4e3 Mon Sep 17 00:00:00 2001 From: danielfeismann Date: Tue, 14 May 2024 13:30:16 +0200 Subject: [PATCH 1/3] Refactor getAllUniqueCombinations avoid nested loops --- CHANGELOG.md | 1 + src/main/scala/utils/Connections.scala | 5 ++-- src/main/scala/utils/OsmoGridUtils.scala | 32 ++++-------------------- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e72b85b2..018d5668 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ 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) +- Refactoring of 'getAllUniqueCombinations' to avoid nested loops [#431](https://github.com/ie3-institute/OSMoGrid/issues/431) ### Fixed - Fixed bug in `LvGridGeneratorSupport` [#388](https://github.com/ie3-institute/OSMoGrid/issues/388) diff --git a/src/main/scala/utils/Connections.scala b/src/main/scala/utils/Connections.scala index 1ca85b63..9df9f9e0 100644 --- a/src/main/scala/utils/Connections.scala +++ b/src/main/scala/utils/Connections.scala @@ -26,6 +26,7 @@ import utils.OsmoGridUtils.getAllUniqueCombinations import javax.measure.quantity.Length import scala.collection.mutable +import scala.collection.parallel.CollectionConverters._ import scala.jdk.CollectionConverters._ /** This utility object contains all known [[Connection]]s. @@ -201,7 +202,7 @@ object Connections { val vertexes = graph.vertexSet().asScala val paths = vertexes.map { v => v -> shortestPath.getPaths(v) }.toMap - getAllUniqueCombinations(graph.vertexSet().asScala.toList).map { + getAllUniqueCombinations(graph.vertexSet().asScala.toList).par.map { case (nodeA, nodeB) => val path = paths(nodeA).getPath(nodeB) @@ -217,7 +218,7 @@ object Connections { Quantities.getQuantity(path.getWeight, Units.METRE), Some(path) ) - } + }.toList } /** Method for creating undirected [[Connection]]s. Excluding connections that diff --git a/src/main/scala/utils/OsmoGridUtils.scala b/src/main/scala/utils/OsmoGridUtils.scala index 735ca9ad..ae4f7711 100644 --- a/src/main/scala/utils/OsmoGridUtils.scala +++ b/src/main/scala/utils/OsmoGridUtils.scala @@ -139,36 +139,14 @@ object OsmoGridUtils { * @return * a list of all unique connections */ - def getAllUniqueCombinations[T]( - nodes: List[T] - ): List[(T, T)] = { + def getAllUniqueCombinations[T](nodes: List[T]): List[(T, T)] = { if (nodes.size < 2) { List.empty - } else if (nodes.size == 2) { - List((nodes(0), nodes(1))) } else { - val connections: util.List[(T, T)] = - new util.ArrayList[(T, T)] - - // algorithm to find all unique combinations - nodes.foreach(nodeA => { - nodes.foreach(nodeB => { - // it makes no sense to connect a node to itself => nodeA and nodeB cannot be the same - if (nodeA != nodeB) { - // two combinations possible - val t1 = (nodeA, nodeB) - val t2 = (nodeB, nodeA) - - // if none of the combinations is already added, the first combination is added - if (!connections.contains(t1) && !connections.contains(t2)) { - connections.add(t1) - } - } - }) - }) - - // returns all unique connections - connections.asScala.toList + for { + (a, i) <- nodes.zipWithIndex + b <- nodes.drop(i + 1) + } yield (a, b) } } From a4a7e05dea8a8bf785fd5a69845b7ce60d5f5a15 Mon Sep 17 00:00:00 2001 From: danielfeismann Date: Tue, 14 May 2024 19:19:01 +0200 Subject: [PATCH 2/3] add logger info --- src/main/scala/utils/Connections.scala | 44 ++++++++++++++++---------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/main/scala/utils/Connections.scala b/src/main/scala/utils/Connections.scala index 9df9f9e0..019c6014 100644 --- a/src/main/scala/utils/Connections.scala +++ b/src/main/scala/utils/Connections.scala @@ -6,6 +6,7 @@ package utils +import com.typesafe.scalalogging.LazyLogging import edu.ie3.datamodel.graph.{DistanceWeightedEdge, DistanceWeightedGraph} import edu.ie3.datamodel.models.input.NodeInput import edu.ie3.datamodel.models.input.connector.LineInput @@ -102,7 +103,7 @@ case class Connections[T]( } } -object Connections { +object Connections extends LazyLogging { val log: Logger = LoggerFactory.getLogger(Connections.getClass) /** Utility object for connections. @@ -202,23 +203,34 @@ object Connections { val vertexes = graph.vertexSet().asScala val paths = vertexes.map { v => v -> shortestPath.getPaths(v) }.toMap - getAllUniqueCombinations(graph.vertexSet().asScala.toList).par.map { - case (nodeA, nodeB) => - val path = paths(nodeA).getPath(nodeB) + if (vertexes.size > 1000) + logger.info( + s"Needs to find shortest path connections for ${vertexes.size} vertices. This may take a while." + ) + val connections = + getAllUniqueCombinations(graph.vertexSet().asScala.toList).par.map { + case (nodeA, nodeB) => + val path = paths(nodeA).getPath(nodeB) + + if (path == null) { + throw GridException( + s"No path could be found between $nodeA and $nodeB, because either node is not connected to the graph." + ) + } - if (path == null) { - throw GridException( - s"No path could be found between $nodeA and $nodeB, because either node is not connected to the graph." + Connection( + nodeA, + nodeB, + Quantities.getQuantity(path.getWeight, Units.METRE), + Some(path) ) - } - - Connection( - nodeA, - nodeB, - Quantities.getQuantity(path.getWeight, Units.METRE), - Some(path) - ) - }.toList + + }.toList + if (vertexes.size > 1000) + logger.info( + s"Finished search for shortest path connections for ${vertexes.size} vertices." + ) + connections } /** Method for creating undirected [[Connection]]s. Excluding connections that From 95cdcd7c374d4804439fc20d2007132bc290c343 Mon Sep 17 00:00:00 2001 From: danielfeismann Date: Mon, 27 May 2024 06:52:20 +0200 Subject: [PATCH 3/3] add explanatory comment to include reviewer feedback --- src/main/scala/utils/OsmoGridUtils.scala | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/scala/utils/OsmoGridUtils.scala b/src/main/scala/utils/OsmoGridUtils.scala index ae4f7711..3fcc1947 100644 --- a/src/main/scala/utils/OsmoGridUtils.scala +++ b/src/main/scala/utils/OsmoGridUtils.scala @@ -8,12 +8,7 @@ package utils import edu.ie3.datamodel.models.OperationTime import edu.ie3.datamodel.models.input.connector.Transformer2WInput -import edu.ie3.datamodel.models.input.container.{ - GraphicElements, - JointGridContainer, - RawGridElements, - SystemParticipants -} +import edu.ie3.datamodel.models.input.container.JointGridContainer import edu.ie3.datamodel.models.input.{NodeInput, OperatorInput} import edu.ie3.datamodel.models.voltagelevels.VoltageLevel import edu.ie3.osmogrid.cfg.OsmoGridConfig.Voltage @@ -34,7 +29,6 @@ import org.locationtech.jts.geom.{Coordinate, Polygon} import tech.units.indriya.ComparableQuantity import tech.units.indriya.unit.Units -import java.util import java.util.UUID import javax.measure.quantity.{Area, Power} import scala.collection.parallel.ParSeq @@ -144,6 +138,9 @@ object OsmoGridUtils { List.empty } else { for { + // Loops through all nodes as a and combines each node a with each b + // of the same nodes list. Nodes as b are restricted to those that + // occur later in the list than each node a respectively. (a, i) <- nodes.zipWithIndex b <- nodes.drop(i + 1) } yield (a, b)