From 0d2ca67e249d42db382ee760e492ec9c5289a819 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:37:33 -0600
Subject: [PATCH 01/20] system_structure_modifier: delete method that has
 warning not to use it

_set_nominal_voltages seems to be the worst of three methods defined to set nominal voltages
---
 ditto/modify/system_structure.py | 79 --------------------------------
 1 file changed, 79 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index b607c28a..5401e46c 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -639,85 +639,6 @@ def set_nominal_voltages(self):
                     ):
                         obj.nominal_voltage = node_from_object.nominal_voltage
 
-    def _set_nominal_voltages(self):
-        """This function looks for all nodes and lines which have empty nominal_voltage fields.
-        The function goes upstream in the network representation to find a transformer.
-        The nominal voltage of the secondary of this transformer is then used to fill the empty nominal_voltage fields.
-
-        .. warning:: DO NOT USE. Use set_nominal_voltages instead
-
-        .. TODO:: Remove this once everything is stable.
-        """
-        self.model.set_names()
-        # Loop over the objects
-        for obj in self.model.models:
-
-            # If we get a Node with an empty nominal_voltage field
-            if isinstance(obj, Node) and obj.nominal_voltage is None:
-                # Get the upstream transformer name
-                try:
-                    upstream_transformer_name = self.G.get_upstream_transformer(
-                        self.model, obj.name
-                    )
-                except:
-                    continue
-                if upstream_transformer_name is not None:
-                    # Get the corresponding object
-                    upstream_transformer_object = self.model[upstream_transformer_name]
-                    # If possible, get all the winding voltages and select the minimum as the secondary voltage
-                    if (
-                        hasattr(upstream_transformer_object, "windings")
-                        and upstream_transformer_object.windings is not None
-                    ):
-                        volts = []
-                        for winding in upstream_transformer_object.windings:
-                            if (
-                                hasattr(winding, "nominal_voltage")
-                                and winding.nominal_voltage is not None
-                            ):
-                                volts.append(winding.nominal_voltage)
-                        secondary_voltage = min(volts)
-                        # Finally, assign this value to the object's nominal voltage
-                        obj.nominal_voltage = secondary_voltage
-
-            # If we get a line
-            if isinstance(obj, Line) and obj.nominal_voltage is None:
-                # Get the from node
-                if hasattr(obj, "from_element") and obj.from_element is not None:
-                    node_from_object = self.model[obj.from_element]
-
-                    # If the from node has a known nominal voltage, then use this value
-                    if (
-                        hasattr(node_from_object, "nominal_voltage")
-                        and node_from_object.nominal_voltage is not None
-                    ):
-                        obj.nominal_voltage = node_from_object.nominal_voltage
-
-                    # Otherwise, do as before with the from node
-                    #
-                    # TODO: Put the following code into a function
-                    #
-                    else:
-                        upstream_transformer_name = self.G.get_upstream_transformer(
-                            self.model, node_from_object.name
-                        )
-                        if upstream_transformer_name is not None:
-                            upstream_transformer_object = self.model[
-                                upstream_transformer_name
-                            ]
-                            if (
-                                hasattr(upstream_transformer_object, "windings")
-                                and upstream_transformer_object.windings is not None
-                            ):
-                                volts = []
-                                for winding in upstream_transformer_object.windings:
-                                    if (
-                                        hasattr(winding, "nominal_voltage")
-                                        and winding.nominal_voltage is not None
-                                    ):
-                                        volts.append(winding.nominal_voltage)
-                                secondary_voltage = min(volts)
-                                obj.nominal_voltage = secondary_voltage
 
     def center_tap_load_preprocessing(self):
         """Performs the center tap load pre-processing step.

From 8a102473bdb2f2a2214cb4ad8c9086948e72dc7c Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:39:08 -0600
Subject: [PATCH 02/20] use the recommended
 system_structure_modifier.set_nominal_voltages_recur in NetworkAnalyzer

---
 ditto/metrics/network_analysis.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index b446b24c..e85a4afb 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -157,7 +157,7 @@ def __init__(self, model, compute_network=True, *args):
             self.edge_equipment_name = None
 
         modifier = system_structure_modifier(self.model, source)
-        modifier.set_nominal_voltages()
+        modifier.set_nominal_voltages_recur()
         # IMPORTANT: the following two parameters define what is LV and what is MV.
         # - Object is LV if object.nominal_voltage<=LV_threshold
         # - Object is MV if MV_threshold>=object.nominal_voltage>LV_threshold

From 3ffe4e4146d319cb1970d92f274c87faea8f940c Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:39:41 -0600
Subject: [PATCH 03/20] refactor `is not ""` to correct `!= ""` in
 NetworkAnalyzer

---
 ditto/metrics/network_analysis.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index e85a4afb..eadde519 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -465,7 +465,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):
@@ -498,7 +498,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):
@@ -525,7 +525,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):

From 4929915eca3839c6238e8d95ad687c6dd4ef0abb Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:40:09 -0600
Subject: [PATCH 04/20] rm broken, unused set_nominal_voltages in
 system_structure_modifier

---
 ditto/modify/system_structure.py | 120 -------------------------------
 1 file changed, 120 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index 5401e46c..b9a6b61b 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -520,126 +520,6 @@ def test_feeder_cut(self):
                         )
                         logger.debug(intersection)
 
-    def set_nominal_voltages(self):
-        """This function does the exact same thing as _set_nominal_voltages.
-        The implementation is less obvious but should be much faster.
-
-        **Algorithm:**
-
-            - Find all edges modeling transformers in the network
-            - Disconnect these edges (which should disconnect the network)
-            - Compute the connected components
-            - Group the nodes according to these connected components
-            - Re-connect the network by adding back the removed edges
-            - For every group of nodes:
-                - Find the nominal voltage of one node (look at secondary voltage of the upstream transformer)
-                - All nodes in this group get the same nominal voltage
-            - For every line:
-                - Set nominal voltage as the nominal voltage of one of the end-points (that we have thanks to the previous loop...)
-
-        .. note:: This should be faster than _set_nominal_voltages since we only look upstream once for every group instead of doing it once for every node.
-
-        .. warning:: Use set_nominal_voltages_recur instead.
-
-        .. TODO:: Find out why the results of this and set_nominal_voltages_recur don't match...
-        """
-        self.model.set_names()
-
-        # We will remove all edges representing transformers
-        edges_to_remove = [
-            edge
-            for edge in self.G.graph.edges(data=True)
-            if "equipment" in edge[2] and edge[2]["equipment"] == "PowerTransformer"
-        ]
-
-        # Do it!!
-        self.G.graph.remove_edges_from(edges_to_remove)
-
-        # Get the connected components
-        cc = nx.connected_components(self.G.graph)
-
-        # Extract the groups of nodes with same nominal voltage
-        node_mapping = [component for component in cc]
-
-        # Restaure the network by addind back the edges previously removed
-        self.G.graph.add_edges_from(edges_to_remove)
-
-        # Graph should be connected, otherwise we broke it...
-        assert nx.is_connected(self.G.graph)
-
-        # Instanciate the list of nominal voltages (one value for each group)
-        nominal_voltage_group = [None for _ in node_mapping]
-        upstream_transformer_name_group = [None for _ in node_mapping]
-
-        # For every group...
-        for idx, group in enumerate(node_mapping):
-
-            # ...first node is volonteered to be searched
-            volonteer = group.pop()
-            while not isinstance(self.model[volonteer], Node):
-                volonteer = group.pop()
-
-            # Get the name of the upstream transformer
-            upstream_transformer_name = self.G.get_upstream_transformer(
-                self.model, volonteer
-            )
-
-            # If we got None, there is nothing we can do. Otherwise...
-            if upstream_transformer_name is not None:
-
-                # ...get the transformer object
-                upstream_transformer_object = self.model[upstream_transformer_name]
-                upstream_transformer_name_group[idx] = upstream_transformer_name
-
-                # Get the nominal voltage of the secondary
-                if (
-                    hasattr(upstream_transformer_object, "windings")
-                    and upstream_transformer_object.windings is not None
-                ):
-
-                    volts = []
-                    for winding in upstream_transformer_object.windings:
-                        if (
-                            hasattr(winding, "nominal_voltage")
-                            and winding.nominal_voltage is not None
-                        ):
-                            volts.append(winding.nominal_voltage)
-
-                    secondary_voltage = min(volts)
-                    # And assign this value as the nominal voltage for the group of nodes
-                    nominal_voltage_group[idx] = secondary_voltage
-
-        # Now, we simply loop over all the groups
-        for idx, group in enumerate(node_mapping):
-
-            # And all the nodes inside of the groups
-            for n in group:
-
-                # And set the nominal voltage as the group value
-                self.model[n].nominal_voltage = nominal_voltage_group[idx]
-                if isinstance(self.model[n], Load):
-                    self.model[
-                        n
-                    ].upstream_transformer_name = upstream_transformer_name_group[idx]
-
-        # Now we take care of the Lines.
-        # Since we should have the nominal voltage for every node (in a perfect world),
-        # We just have to grab the nominal voltage of one of the end-points.
-        for obj in self.model.models:
-            # If we get a line
-            if isinstance(obj, Line) and obj.nominal_voltage is None:
-                # Get the from node
-                if hasattr(obj, "from_element") and obj.from_element is not None:
-                    node_from_object = self.model[obj.from_element]
-
-                    # If the from node has a known nominal voltage, then use this value
-                    if (
-                        hasattr(node_from_object, "nominal_voltage")
-                        and node_from_object.nominal_voltage is not None
-                    ):
-                        obj.nominal_voltage = node_from_object.nominal_voltage
-
-
     def center_tap_load_preprocessing(self):
         """Performs the center tap load pre-processing step.
         This function is responsible for setting the correct phase of center-tap loads.

From 10158224e1e8939de8f8d18e9b7f3ce7358786e6 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:40:36 -0600
Subject: [PATCH 05/20] minor formatting in system_structure_modifier

---
 ditto/modify/system_structure.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index b9a6b61b..8a92bcb8 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -224,9 +224,10 @@ def set_nominal_voltages_recur(self, *args):
             previous = self.source
         else:
             node, voltage, previous = args
-        if (previous, node) in self.edge_equipment and self.edge_equipment[
-            (previous, node)
-        ] == "PowerTransformer":
+        if (
+            (previous, node) in self.edge_equipment and 
+            self.edge_equipment[(previous, node)] == "PowerTransformer"
+        ):
             trans_name = self.edge_equipment_name[(previous, node)]
             new_value = min(
                 [
@@ -235,9 +236,10 @@ def set_nominal_voltages_recur(self, *args):
                     if w.nominal_voltage is not None
                 ]
             )
-        elif (node, previous) in self.edge_equipment and self.edge_equipment[
-            (node, previous)
-        ] == "PowerTransformer":
+        elif (
+            (node, previous) in self.edge_equipment and 
+            self.edge_equipment[(node, previous)] == "PowerTransformer"
+        ):
             trans_name = self.edge_equipment_name[(node, previous)]
             new_value = min(
                 [
@@ -248,6 +250,7 @@ def set_nominal_voltages_recur(self, *args):
             )
         else:
             new_value = voltage
+        
         if hasattr(self.model[node], "nominal_voltage"):
             self.model[node].nominal_voltage = new_value
         for child in self.G.digraph.successors(node):

From 0aca2f5c10e075751e55f6da2e57dd7b242cab88 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:56:28 -0600
Subject: [PATCH 06/20] add debug msg when changing node nominal_voltage in
 system_structure_modifier

---
 ditto/modify/system_structure.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index 8a92bcb8..7d6274ff 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -252,7 +252,9 @@ def set_nominal_voltages_recur(self, *args):
             new_value = voltage
         
         if hasattr(self.model[node], "nominal_voltage"):
-            self.model[node].nominal_voltage = new_value
+            if new_value != self.model[node].nominal_voltage:
+                logger.debug(f"Setting node {node} nominal voltage to {new_value} from {self.model[node].nominal_voltage}")
+                self.model[node].nominal_voltage = new_value
         for child in self.G.digraph.successors(node):
             self.set_nominal_voltages_recur(child, new_value, node)
 

From 0f0a3a5ecc0d6f71df9fc6630d31fb1cd8685a17 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 11:43:34 -0600
Subject: [PATCH 07/20] rm unused arg from Network.get_upstream_transformer

---
 ditto/network/network.py | 2 +-
 ditto/store.py           | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index 8133b572..dc23d51c 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -346,7 +346,7 @@ def remove_open_switches(self, model):
                     if self.digraph.has_edge(m.to_element, m.from_element):
                         self.digraph.remove_edge(m.to_element, m.from_element)
 
-    def get_upstream_transformer(self, model, node):
+    def get_upstream_transformer(self, node):
 
         curr_node = node
         curr = list(self.digraph.predecessors(node))
diff --git a/ditto/store.py b/ditto/store.py
index e6f53b7f..47663ec2 100644
--- a/ditto/store.py
+++ b/ditto/store.py
@@ -187,9 +187,7 @@ def set_node_voltages(self):
         self.set_names()
         for i in self.models:
             if isinstance(i, Node) and hasattr(i, "name") and i.name is not None:
-                upstream_transformer = self._network.get_upstream_transformer(
-                    self, i.name
-                )
+                upstream_transformer = self._network.get_upstream_transformer(i.name)
                 try:
                     upstream_voltage = (
                         self[upstream_transformer].windings[-1].nominal_voltage

From 499d9516018835249fc27912ec63acef15dd68d0 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 11:44:01 -0600
Subject: [PATCH 08/20] update network_analysis.py doc string

---
 ditto/metrics/network_analysis.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index eadde519..f5dc1d6f 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -205,12 +205,10 @@ def add_feeder_information(
         """
         Use this function to add the feeder information if available.
 
-        :param feeder_names: List of the feeder names
-        :type feeder_names: List(str)
-        :param feeder_nodes: List of lists containing feeder nodes
-        :type feeder_nodes: List of Lists of strings
-        :param feeder_types: List of feeder types.
-        :type feeder_types: List or string if all feeders have the same type
+        :param feeder_names: List(str) of the feeder names
+        :param feeder_nodes: List of lists of strings containing feeder nodes
+        :param substations: List(str) of the substations names
+        :param feeder_types: List(str) of feeder types or string if all feeders have the same type
         """
         if len(feeder_names) != len(feeder_nodes):
             raise ValueError(

From 642a8bfdda2cb7a50830f92cd28f61531da860c9 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 13:34:09 -0600
Subject: [PATCH 09/20] minor formatting in Network.__init__

---
 ditto/network/network.py | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index dc23d51c..04f252dd 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -15,17 +15,11 @@
 
 class Network:
     def __init__(self):
-        self.graph = None
-        self.digraph = None  # Doesn't contain attributes, just topology
-        self.class_map = (
-            {}
-        )  # Map the networkx names to the object type (not included in attributes)
-        self.is_built = (
-            False  # Flag that indicates whether the Network has been built or not.
-        )
-        self.attributes_set = (
-            False  # Flag that indicates whether the attributes have been set or not.
-        )
+        self.graph = None  # becomes nx.Graph() in self.build
+        self.digraph = None  # becomes nx.DiGraph() in self.build. Doesn't contain attributes, just topology
+        self.class_map = {} # Map the networkx names to the object type (not included in attributes)
+        self.is_built = False  # Flag that indicates whether the Network has been built or not.
+        self.attributes_set = False  # Flag that indicates whether the attributes have been set or not.
 
     def provide_graphs(self, graph, digraph):
         """

From 8e00e706779b9a9f17643c727f19a2caba413cb3 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 13:35:58 -0600
Subject: [PATCH 10/20] make warnings logger.warn (were logger.debug)

---
 ditto/network/network.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index 04f252dd..7bdd27ca 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -363,21 +363,19 @@ def get_all_elements_downstream(self, model, source):
         model.set_names()
 
         # Checking that the network is already built
-        # TODO: Log instead of printing...
         if not self.is_built:
-            logger.debug(
+            logger.warn(
                 "Warning. Trying to use Network model without building the network."
             )
-            logger.debug("Calling build() with source={}".format(source))
+            logger.warn("Calling build() with source={}".format(source))
             self.build(model, source=source)
 
         # Checking that the attributes have been set
-        # TODO: Log instead of printing...
         if not self.attributes_set:
-            logger.debug(
+            logger.warn(
                 "Warning. Trying to use Network model without setting the attributes first."
             )
-            logger.debug("Setting the attributes...")
+            logger.warn("Setting the attributes...")
             self.set_attributes(model)
 
         # Run the dfs or die trying...

From 7ff8a180a397d7546bdcfa8633391fa5839ea4a5 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:37:33 -0600
Subject: [PATCH 11/20] system_structure_modifier: delete method that has
 warning not to use it

_set_nominal_voltages seems to be the worst of three methods defined to set nominal voltages
---
 ditto/modify/system_structure.py | 79 --------------------------------
 1 file changed, 79 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index b607c28a..5401e46c 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -639,85 +639,6 @@ def set_nominal_voltages(self):
                     ):
                         obj.nominal_voltage = node_from_object.nominal_voltage
 
-    def _set_nominal_voltages(self):
-        """This function looks for all nodes and lines which have empty nominal_voltage fields.
-        The function goes upstream in the network representation to find a transformer.
-        The nominal voltage of the secondary of this transformer is then used to fill the empty nominal_voltage fields.
-
-        .. warning:: DO NOT USE. Use set_nominal_voltages instead
-
-        .. TODO:: Remove this once everything is stable.
-        """
-        self.model.set_names()
-        # Loop over the objects
-        for obj in self.model.models:
-
-            # If we get a Node with an empty nominal_voltage field
-            if isinstance(obj, Node) and obj.nominal_voltage is None:
-                # Get the upstream transformer name
-                try:
-                    upstream_transformer_name = self.G.get_upstream_transformer(
-                        self.model, obj.name
-                    )
-                except:
-                    continue
-                if upstream_transformer_name is not None:
-                    # Get the corresponding object
-                    upstream_transformer_object = self.model[upstream_transformer_name]
-                    # If possible, get all the winding voltages and select the minimum as the secondary voltage
-                    if (
-                        hasattr(upstream_transformer_object, "windings")
-                        and upstream_transformer_object.windings is not None
-                    ):
-                        volts = []
-                        for winding in upstream_transformer_object.windings:
-                            if (
-                                hasattr(winding, "nominal_voltage")
-                                and winding.nominal_voltage is not None
-                            ):
-                                volts.append(winding.nominal_voltage)
-                        secondary_voltage = min(volts)
-                        # Finally, assign this value to the object's nominal voltage
-                        obj.nominal_voltage = secondary_voltage
-
-            # If we get a line
-            if isinstance(obj, Line) and obj.nominal_voltage is None:
-                # Get the from node
-                if hasattr(obj, "from_element") and obj.from_element is not None:
-                    node_from_object = self.model[obj.from_element]
-
-                    # If the from node has a known nominal voltage, then use this value
-                    if (
-                        hasattr(node_from_object, "nominal_voltage")
-                        and node_from_object.nominal_voltage is not None
-                    ):
-                        obj.nominal_voltage = node_from_object.nominal_voltage
-
-                    # Otherwise, do as before with the from node
-                    #
-                    # TODO: Put the following code into a function
-                    #
-                    else:
-                        upstream_transformer_name = self.G.get_upstream_transformer(
-                            self.model, node_from_object.name
-                        )
-                        if upstream_transformer_name is not None:
-                            upstream_transformer_object = self.model[
-                                upstream_transformer_name
-                            ]
-                            if (
-                                hasattr(upstream_transformer_object, "windings")
-                                and upstream_transformer_object.windings is not None
-                            ):
-                                volts = []
-                                for winding in upstream_transformer_object.windings:
-                                    if (
-                                        hasattr(winding, "nominal_voltage")
-                                        and winding.nominal_voltage is not None
-                                    ):
-                                        volts.append(winding.nominal_voltage)
-                                secondary_voltage = min(volts)
-                                obj.nominal_voltage = secondary_voltage
 
     def center_tap_load_preprocessing(self):
         """Performs the center tap load pre-processing step.

From 5672f8390d1a96f1b806d01adc0922a9d8e1447a Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:39:08 -0600
Subject: [PATCH 12/20] use the recommended
 system_structure_modifier.set_nominal_voltages_recur in NetworkAnalyzer

---
 ditto/metrics/network_analysis.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index b446b24c..e85a4afb 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -157,7 +157,7 @@ def __init__(self, model, compute_network=True, *args):
             self.edge_equipment_name = None
 
         modifier = system_structure_modifier(self.model, source)
-        modifier.set_nominal_voltages()
+        modifier.set_nominal_voltages_recur()
         # IMPORTANT: the following two parameters define what is LV and what is MV.
         # - Object is LV if object.nominal_voltage<=LV_threshold
         # - Object is MV if MV_threshold>=object.nominal_voltage>LV_threshold

From 0fa7227bac4df799d8f7bee412402230e1cab25f Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:39:41 -0600
Subject: [PATCH 13/20] refactor `is not ""` to correct `!= ""` in
 NetworkAnalyzer

---
 ditto/metrics/network_analysis.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index e85a4afb..eadde519 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -465,7 +465,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):
@@ -498,7 +498,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):
@@ -525,7 +525,7 @@ def tag_objects(self):
                                 hasattr(prev_obj, "feeder_name")
                                 and hasattr(prev_obj, "name")
                                 and prev_obj.feeder_name is not None
-                                and prev_obj.feeder_name is not ""
+                                and prev_obj.feeder_name != ""
                                 and prev_obj.name
                                 in self.node_feeder_mapping  # In case a default value has been set for all feeder_name values
                             ):

From f3196873bd3ed9f37018c0648d3241b393d9c980 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:40:09 -0600
Subject: [PATCH 14/20] rm broken, unused set_nominal_voltages in
 system_structure_modifier

---
 ditto/modify/system_structure.py | 120 -------------------------------
 1 file changed, 120 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index 5401e46c..b9a6b61b 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -520,126 +520,6 @@ def test_feeder_cut(self):
                         )
                         logger.debug(intersection)
 
-    def set_nominal_voltages(self):
-        """This function does the exact same thing as _set_nominal_voltages.
-        The implementation is less obvious but should be much faster.
-
-        **Algorithm:**
-
-            - Find all edges modeling transformers in the network
-            - Disconnect these edges (which should disconnect the network)
-            - Compute the connected components
-            - Group the nodes according to these connected components
-            - Re-connect the network by adding back the removed edges
-            - For every group of nodes:
-                - Find the nominal voltage of one node (look at secondary voltage of the upstream transformer)
-                - All nodes in this group get the same nominal voltage
-            - For every line:
-                - Set nominal voltage as the nominal voltage of one of the end-points (that we have thanks to the previous loop...)
-
-        .. note:: This should be faster than _set_nominal_voltages since we only look upstream once for every group instead of doing it once for every node.
-
-        .. warning:: Use set_nominal_voltages_recur instead.
-
-        .. TODO:: Find out why the results of this and set_nominal_voltages_recur don't match...
-        """
-        self.model.set_names()
-
-        # We will remove all edges representing transformers
-        edges_to_remove = [
-            edge
-            for edge in self.G.graph.edges(data=True)
-            if "equipment" in edge[2] and edge[2]["equipment"] == "PowerTransformer"
-        ]
-
-        # Do it!!
-        self.G.graph.remove_edges_from(edges_to_remove)
-
-        # Get the connected components
-        cc = nx.connected_components(self.G.graph)
-
-        # Extract the groups of nodes with same nominal voltage
-        node_mapping = [component for component in cc]
-
-        # Restaure the network by addind back the edges previously removed
-        self.G.graph.add_edges_from(edges_to_remove)
-
-        # Graph should be connected, otherwise we broke it...
-        assert nx.is_connected(self.G.graph)
-
-        # Instanciate the list of nominal voltages (one value for each group)
-        nominal_voltage_group = [None for _ in node_mapping]
-        upstream_transformer_name_group = [None for _ in node_mapping]
-
-        # For every group...
-        for idx, group in enumerate(node_mapping):
-
-            # ...first node is volonteered to be searched
-            volonteer = group.pop()
-            while not isinstance(self.model[volonteer], Node):
-                volonteer = group.pop()
-
-            # Get the name of the upstream transformer
-            upstream_transformer_name = self.G.get_upstream_transformer(
-                self.model, volonteer
-            )
-
-            # If we got None, there is nothing we can do. Otherwise...
-            if upstream_transformer_name is not None:
-
-                # ...get the transformer object
-                upstream_transformer_object = self.model[upstream_transformer_name]
-                upstream_transformer_name_group[idx] = upstream_transformer_name
-
-                # Get the nominal voltage of the secondary
-                if (
-                    hasattr(upstream_transformer_object, "windings")
-                    and upstream_transformer_object.windings is not None
-                ):
-
-                    volts = []
-                    for winding in upstream_transformer_object.windings:
-                        if (
-                            hasattr(winding, "nominal_voltage")
-                            and winding.nominal_voltage is not None
-                        ):
-                            volts.append(winding.nominal_voltage)
-
-                    secondary_voltage = min(volts)
-                    # And assign this value as the nominal voltage for the group of nodes
-                    nominal_voltage_group[idx] = secondary_voltage
-
-        # Now, we simply loop over all the groups
-        for idx, group in enumerate(node_mapping):
-
-            # And all the nodes inside of the groups
-            for n in group:
-
-                # And set the nominal voltage as the group value
-                self.model[n].nominal_voltage = nominal_voltage_group[idx]
-                if isinstance(self.model[n], Load):
-                    self.model[
-                        n
-                    ].upstream_transformer_name = upstream_transformer_name_group[idx]
-
-        # Now we take care of the Lines.
-        # Since we should have the nominal voltage for every node (in a perfect world),
-        # We just have to grab the nominal voltage of one of the end-points.
-        for obj in self.model.models:
-            # If we get a line
-            if isinstance(obj, Line) and obj.nominal_voltage is None:
-                # Get the from node
-                if hasattr(obj, "from_element") and obj.from_element is not None:
-                    node_from_object = self.model[obj.from_element]
-
-                    # If the from node has a known nominal voltage, then use this value
-                    if (
-                        hasattr(node_from_object, "nominal_voltage")
-                        and node_from_object.nominal_voltage is not None
-                    ):
-                        obj.nominal_voltage = node_from_object.nominal_voltage
-
-
     def center_tap_load_preprocessing(self):
         """Performs the center tap load pre-processing step.
         This function is responsible for setting the correct phase of center-tap loads.

From 9a5e238e5b10cc6002cdeaf7aaada35bf811d7c5 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:40:36 -0600
Subject: [PATCH 15/20] minor formatting in system_structure_modifier

---
 ditto/modify/system_structure.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index b9a6b61b..8a92bcb8 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -224,9 +224,10 @@ def set_nominal_voltages_recur(self, *args):
             previous = self.source
         else:
             node, voltage, previous = args
-        if (previous, node) in self.edge_equipment and self.edge_equipment[
-            (previous, node)
-        ] == "PowerTransformer":
+        if (
+            (previous, node) in self.edge_equipment and 
+            self.edge_equipment[(previous, node)] == "PowerTransformer"
+        ):
             trans_name = self.edge_equipment_name[(previous, node)]
             new_value = min(
                 [
@@ -235,9 +236,10 @@ def set_nominal_voltages_recur(self, *args):
                     if w.nominal_voltage is not None
                 ]
             )
-        elif (node, previous) in self.edge_equipment and self.edge_equipment[
-            (node, previous)
-        ] == "PowerTransformer":
+        elif (
+            (node, previous) in self.edge_equipment and 
+            self.edge_equipment[(node, previous)] == "PowerTransformer"
+        ):
             trans_name = self.edge_equipment_name[(node, previous)]
             new_value = min(
                 [
@@ -248,6 +250,7 @@ def set_nominal_voltages_recur(self, *args):
             )
         else:
             new_value = voltage
+        
         if hasattr(self.model[node], "nominal_voltage"):
             self.model[node].nominal_voltage = new_value
         for child in self.G.digraph.successors(node):

From 3be955866e9e716526ee71a5e0c4c8be5ea14bd0 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 10:56:28 -0600
Subject: [PATCH 16/20] add debug msg when changing node nominal_voltage in
 system_structure_modifier

---
 ditto/modify/system_structure.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ditto/modify/system_structure.py b/ditto/modify/system_structure.py
index 8a92bcb8..7d6274ff 100644
--- a/ditto/modify/system_structure.py
+++ b/ditto/modify/system_structure.py
@@ -252,7 +252,9 @@ def set_nominal_voltages_recur(self, *args):
             new_value = voltage
         
         if hasattr(self.model[node], "nominal_voltage"):
-            self.model[node].nominal_voltage = new_value
+            if new_value != self.model[node].nominal_voltage:
+                logger.debug(f"Setting node {node} nominal voltage to {new_value} from {self.model[node].nominal_voltage}")
+                self.model[node].nominal_voltage = new_value
         for child in self.G.digraph.successors(node):
             self.set_nominal_voltages_recur(child, new_value, node)
 

From 06ba51b92142bd3e774e0c7e897b6870294b61e1 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 11:43:34 -0600
Subject: [PATCH 17/20] rm unused arg from Network.get_upstream_transformer

---
 ditto/network/network.py | 2 +-
 ditto/store.py           | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index 8133b572..dc23d51c 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -346,7 +346,7 @@ def remove_open_switches(self, model):
                     if self.digraph.has_edge(m.to_element, m.from_element):
                         self.digraph.remove_edge(m.to_element, m.from_element)
 
-    def get_upstream_transformer(self, model, node):
+    def get_upstream_transformer(self, node):
 
         curr_node = node
         curr = list(self.digraph.predecessors(node))
diff --git a/ditto/store.py b/ditto/store.py
index e6f53b7f..47663ec2 100644
--- a/ditto/store.py
+++ b/ditto/store.py
@@ -187,9 +187,7 @@ def set_node_voltages(self):
         self.set_names()
         for i in self.models:
             if isinstance(i, Node) and hasattr(i, "name") and i.name is not None:
-                upstream_transformer = self._network.get_upstream_transformer(
-                    self, i.name
-                )
+                upstream_transformer = self._network.get_upstream_transformer(i.name)
                 try:
                     upstream_voltage = (
                         self[upstream_transformer].windings[-1].nominal_voltage

From 9300e4567943b5fd3241ddee3a8e9b9d3499dfe7 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 11:44:01 -0600
Subject: [PATCH 18/20] update network_analysis.py doc string

---
 ditto/metrics/network_analysis.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ditto/metrics/network_analysis.py b/ditto/metrics/network_analysis.py
index eadde519..f5dc1d6f 100644
--- a/ditto/metrics/network_analysis.py
+++ b/ditto/metrics/network_analysis.py
@@ -205,12 +205,10 @@ def add_feeder_information(
         """
         Use this function to add the feeder information if available.
 
-        :param feeder_names: List of the feeder names
-        :type feeder_names: List(str)
-        :param feeder_nodes: List of lists containing feeder nodes
-        :type feeder_nodes: List of Lists of strings
-        :param feeder_types: List of feeder types.
-        :type feeder_types: List or string if all feeders have the same type
+        :param feeder_names: List(str) of the feeder names
+        :param feeder_nodes: List of lists of strings containing feeder nodes
+        :param substations: List(str) of the substations names
+        :param feeder_types: List(str) of feeder types or string if all feeders have the same type
         """
         if len(feeder_names) != len(feeder_nodes):
             raise ValueError(

From 411c51806427b50701ef4c888ebf544510ce8484 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 13:34:09 -0600
Subject: [PATCH 19/20] minor formatting in Network.__init__

---
 ditto/network/network.py | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index dc23d51c..04f252dd 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -15,17 +15,11 @@
 
 class Network:
     def __init__(self):
-        self.graph = None
-        self.digraph = None  # Doesn't contain attributes, just topology
-        self.class_map = (
-            {}
-        )  # Map the networkx names to the object type (not included in attributes)
-        self.is_built = (
-            False  # Flag that indicates whether the Network has been built or not.
-        )
-        self.attributes_set = (
-            False  # Flag that indicates whether the attributes have been set or not.
-        )
+        self.graph = None  # becomes nx.Graph() in self.build
+        self.digraph = None  # becomes nx.DiGraph() in self.build. Doesn't contain attributes, just topology
+        self.class_map = {} # Map the networkx names to the object type (not included in attributes)
+        self.is_built = False  # Flag that indicates whether the Network has been built or not.
+        self.attributes_set = False  # Flag that indicates whether the attributes have been set or not.
 
     def provide_graphs(self, graph, digraph):
         """

From 6b4396d728c88559ed070bd7a955cad95ce2eca7 Mon Sep 17 00:00:00 2001
From: Nick Laws <nlaws@camus.energy>
Date: Tue, 20 Sep 2022 13:35:58 -0600
Subject: [PATCH 20/20] make warnings logger.warn (were logger.debug)

---
 ditto/network/network.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ditto/network/network.py b/ditto/network/network.py
index 04f252dd..7bdd27ca 100644
--- a/ditto/network/network.py
+++ b/ditto/network/network.py
@@ -363,21 +363,19 @@ def get_all_elements_downstream(self, model, source):
         model.set_names()
 
         # Checking that the network is already built
-        # TODO: Log instead of printing...
         if not self.is_built:
-            logger.debug(
+            logger.warn(
                 "Warning. Trying to use Network model without building the network."
             )
-            logger.debug("Calling build() with source={}".format(source))
+            logger.warn("Calling build() with source={}".format(source))
             self.build(model, source=source)
 
         # Checking that the attributes have been set
-        # TODO: Log instead of printing...
         if not self.attributes_set:
-            logger.debug(
+            logger.warn(
                 "Warning. Trying to use Network model without setting the attributes first."
             )
-            logger.debug("Setting the attributes...")
+            logger.warn("Setting the attributes...")
             self.set_attributes(model)
 
         # Run the dfs or die trying...