Merge pull request #467 from SpiNNakerManchester/coverage

Increase coverage and some minor fixes.

Author: Christian-B

pacman/model/graphs/abstract_single_source_partition.py

@@ -35,10 +35,10 @@ def __init__(
 
     @overrides(AbstractEdgePartition.add_edge)
     def add_edge(self, edge):
+        super().add_edge(edge)
         if edge.pre_vertex != self._pre_vertex:
             raise PacmanConfigurationException(
                 "A partition can only contain edges with the same pre_vertex")
-        super().add_edge(edge)
 
     @property
     def pre_vertex(self):

pacman/model/graphs/application/application_fpga_vertex.py

@@ -106,8 +106,9 @@ def incoming_fpga_connections(self):
 
         :rtype: iter(FPGAConnection)
         """
-        for conn in self._incoming_fpga_connections:
-            yield from conn.expanded
+        if self._incoming_fpga_connections:
+            for conn in self._incoming_fpga_connections:
+                yield from conn.expanded
 
     @property
     def outgoing_fpga_connection(self):

pacman/model/graphs/application/application_graph.py

@@ -102,15 +102,15 @@ def add_edge(self, edge, outgoing_edge_partition_name):
             added to this partition that start at a different vertex to this
             one
         """
+        self._check_edge(edge)
         # Add the edge to the partition
         partition = self.get_outgoing_edge_partition_starting_at_vertex(
             edge.pre_vertex, outgoing_edge_partition_name)
         if partition is None:
             partition = ApplicationEdgePartition(
                 identifier=outgoing_edge_partition_name,
                 pre_vertex=edge.pre_vertex)
-            self.add_outgoing_edge_partition(partition)
-        self._check_edge(edge)
+            self._add_outgoing_edge_partition(partition)
         partition.add_edge(edge)
         return partition
 
@@ -151,50 +151,17 @@ def edges(self):
             for partition in self.outgoing_edge_partitions
             for edge in partition.edges]
 
-    def add_outgoing_edge_partition(self, edge_partition):
+    def _add_outgoing_edge_partition(self, edge_partition):
         """ Add an edge partition to the graph.
 
         Will also add any edges already in the partition as well
 
         :param ApplicationEdgePartition edge_partition:
-            The edge partition to add
-        :raises PacmanAlreadyExistsException:
-            If a partition already exists with the same pre_vertex and
-            identifier
         """
-        # verify that this partition is suitable for this graph
-        if not isinstance(edge_partition, ApplicationEdgePartition):
-            raise PacmanInvalidParameterException(
-                "outgoing_edge_partition", edge_partition.__class__,
-                "Partitions must be an ApplicationEdgePartition")
-
-        # check this partition doesn't already exist
-        key = (edge_partition.pre_vertex,
-               edge_partition.identifier)
-        if edge_partition in self._outgoing_edge_partitions_by_pre_vertex[
-                edge_partition.pre_vertex]:
-            raise PacmanAlreadyExistsException(
-                str(ApplicationEdgePartition), key)
-
         self._outgoing_edge_partitions_by_pre_vertex[
             edge_partition.pre_vertex].add(edge_partition)
-        for edge in edge_partition.edges:
-            self._check_edge(edge)
-
         self._n_outgoing_edge_partitions += 1
 
-    def get_edges_starting_at_vertex(self, vertex):
-        """ Get all the edges that start at the given vertex.
-
-        :param AbstractVertex vertex:
-            The vertex at which the edges to get start
-        :rtype: iterable(AbstractEdge)
-        """
-        parts = self.get_outgoing_edge_partitions_starting_at_vertex(vertex)
-        for partition in parts:
-            for edge in partition.edges:
-                yield edge
-
     @property
     def outgoing_edge_partitions(self):
         """ The edge partitions in the graph.

pacman/model/graphs/machine/constant_sdram_machine_partition.py

@@ -85,6 +85,7 @@ def get_sdram_base_address_for(self, vertex):
 
     @overrides(AbstractSDRAMPartition.get_sdram_size_of_region_for)
     def get_sdram_size_of_region_for(self, vertex):
-        if len(self._edges) == 0:
-            return 0
-        return self._edges.peek().sdram_size
+        if self._sdram_size is None:
+            raise PartitionMissingEdgesException(
+                self.MISSING_EDGE_ERROR_MESSAGE.format(self))
+        return self._sdram_size

pacman/model/graphs/machine/destination_segmented_sdram_machine_partition.py

@@ -15,7 +15,8 @@
 
 from spinn_utilities.overrides import overrides
 from pacman.exceptions import (
-    PacmanConfigurationException,  PartitionMissingEdgesException)
+    PacmanConfigurationException,  PartitionMissingEdgesException,
+    PacmanValueError)
 from pacman.model.graphs import AbstractSingleSourcePartition
 from pacman.model.graphs.machine import (
     AbstractSDRAMPartition, SDRAMMachineEdge)
@@ -77,7 +78,7 @@ def get_sdram_base_address_for(self, vertex):
         for edge in self._edges:
             if edge.post_vertex == vertex:
                 return edge.sdram_base_address
-        return None
+        raise PacmanValueError(f"Vertex {vertex} is not in this partition")
 
     @overrides(AbstractSDRAMPartition.get_sdram_size_of_region_for)
     def get_sdram_size_of_region_for(self, vertex):
@@ -86,4 +87,4 @@ def get_sdram_size_of_region_for(self, vertex):
         for edge in self._edges:
             if edge.post_vertex == vertex:
                 return edge.sdram_size
-        return None
+        raise PacmanValueError(f"Vertex {vertex} is not in this partition")

pacman/model/graphs/machine/source_segmented_sdram_machine_partition.py

@@ -55,52 +55,52 @@ def sdram_base_address(self):
 
     @overrides(AbstractMultiplePartition.add_edge)
     def add_edge(self, edge):
-        # add
-        super().add_edge(edge)
-
         # check
-        if len(self._destinations.keys()) != 1:
+        if len(self._destinations):
+            if edge.post_vertex not in self._destinations:
+                raise PacmanConfigurationException(
+                    "The {} can only support 1 destination vertex".format(
+                        self.__class__.__name__))
+        try:
+            if len(self._pre_vertices[edge.pre_vertex]) != 0:
+                raise PacmanConfigurationException(
+                    f"The {self.__class__.__name__} only supports 1 edge from "
+                    f"a given pre vertex.")
+        except KeyError as ex:
             raise PacmanConfigurationException(
-                "The {} can only support 1 destination vertex".format(
-                    self.__class__.__name__))
-        if len(self._pre_vertices[edge.pre_vertex]) != 1:
-            raise PacmanConfigurationException(
-                "The {} only supports 1 edge from a given pre vertex.".format(
-                    self.__class__.__name__))
-
-        if self._sdram_base_address is not None:
-            raise PacmanConfigurationException(
-                "Illegal attempt to add an edge after sdram_base_address set")
+                "Edge pre_vertex is not a Partition. pre vertex") from ex
+        # add
+        super().add_edge(edge)
 
     @sdram_base_address.setter
     def sdram_base_address(self, new_value):
+        if len(self.pre_vertices) != len(self.edges):
+            raise PartitionMissingEdgesException(
+                f"There are {len(self.pre_vertices)} pre vertices "
+                f"but only {len(self.edges)} edges")
+
         self._sdram_base_address = new_value
 
         for pre_vertex in self._pre_vertices.keys():
-            try:
-                # allocate for the pre_vertex
-                edge = self._pre_vertices[pre_vertex].peek()
-                edge.sdram_base_address = new_value
-                new_value += edge.sdram_size
-            except KeyError as e:
-                raise PartitionMissingEdgesException(
-                    "This partition has no edge from {}".format(
-                        pre_vertex)) from e
+            # allocate for the pre_vertex
+            edge = self._pre_vertices[pre_vertex].peek()
+            edge.sdram_base_address = new_value
+            new_value += edge.sdram_size
 
     @overrides(AbstractSDRAMPartition.get_sdram_base_address_for)
     def get_sdram_base_address_for(self, vertex):
+        if self._sdram_base_address is None:
+            return None
         if vertex in self._pre_vertices:
-            for edge in self._edges:
-                if edge.pre_vertex == vertex:
-                    return edge.sdram_base_address
+            edge = self._pre_vertices[vertex].peek()
+            return edge.sdram_base_address
         else:
             return self._sdram_base_address
 
     @overrides(AbstractSDRAMPartition.get_sdram_size_of_region_for)
     def get_sdram_size_of_region_for(self, vertex):
         if vertex in self._pre_vertices:
-            for edge in self._edges:
-                if edge.pre_vertex == vertex:
-                    return edge.sdram_size
+            edge = self._pre_vertices[vertex].peek()
+            return edge.sdram_size
         else:
             return self.total_sdram_requirements()

pacman/model/partitioner_splitters/splitter_external_device.py

@@ -48,23 +48,22 @@ def set_governed_app_vertex(self, app_vertex):
 
         if isinstance(app_vertex, ApplicationFPGAVertex):
             # This can have multiple FPGA connections per board
-            if app_vertex.incoming_fpga_connections:
-                for fpga in app_vertex.incoming_fpga_connections:
-                    for i in range(app_vertex.n_machine_vertices_per_link):
-                        label = (
-                            f"Incoming Machine vertex {i} for"
-                            f" {app_vertex.label}"
-                            f":{fpga.fpga_id}:{fpga.fpga_link_id}"
-                            f":{fpga.board_address}:{fpga.chip_coords}")
-                        vertex_slice = app_vertex.get_incoming_slice_for_link(
-                            fpga, i)
-                        vertex = MachineFPGAVertex(
-                            fpga.fpga_id, fpga.fpga_link_id,
-                            fpga.board_address, fpga.chip_coords, label=label,
-                            app_vertex=app_vertex, vertex_slice=vertex_slice,
-                            incoming=True, outgoing=False)
-                        self.__incoming_vertices.append(vertex)
-                        self.__incoming_slices.append(vertex_slice)
+            for fpga in app_vertex.incoming_fpga_connections:
+                for i in range(app_vertex.n_machine_vertices_per_link):
+                    label = (
+                        f"Incoming Machine vertex {i} for"
+                        f" {app_vertex.label}"
+                        f":{fpga.fpga_id}:{fpga.fpga_link_id}"
+                        f":{fpga.board_address}:{fpga.chip_coords}")
+                    vertex_slice = app_vertex.get_incoming_slice_for_link(
+                        fpga, i)
+                    vertex = MachineFPGAVertex(
+                        fpga.fpga_id, fpga.fpga_link_id,
+                        fpga.board_address, fpga.chip_coords, label=label,
+                        app_vertex=app_vertex, vertex_slice=vertex_slice,
+                        incoming=True, outgoing=False)
+                    self.__incoming_vertices.append(vertex)
+                    self.__incoming_slices.append(vertex_slice)
             fpga = app_vertex.outgoing_fpga_connection
             if fpga is not None:
                 label = (

pacman/model/resources/multi_region_sdram.py

@@ -83,7 +83,12 @@ def nest(self, region, other):
             self._per_timestep_sdram + other.per_timestep
         if region in self.__regions:
             if isinstance(other, MultiRegionSDRAM):
-                self.__regions[region].merge(other)
+                if isinstance(self.__regions[region], MultiRegionSDRAM):
+                    self.__regions[region].merge(other)
+                else:
+                    old = self.__regions[region]
+                    other.add_cost(region, old.fixed, old.per_timestep)
+                    self.__regions[region] = other
             else:
                 self.__regions[region] += other
         else:

pacman/model/resources/reverse_iptag_resource.py

@@ -87,6 +87,8 @@ def __repr__(self):
         )
 
     def __eq__(self, other):
+        if not isinstance(other, ReverseIPtagResource):
+            return False
         return (self._port == other._port and
                 self._sdp_port == other._sdp_port and
                 self._tag == other._tag)

pacman/operations/routing_table_generators/merged_routing_table_generator.py

@@ -55,14 +55,14 @@ def __create_routing_table(x, y, partitions_in_table, routing_info):
     table = UnCompressedMulticastRoutingTable(x, y)
     iterator = _IteratorWithNext(partitions_in_table.items())
     while iterator.has_next:
-        (vertex, part_id), entry = iterator.next
+        (vertex, part_id), entry = iterator.pop()
         r_info = routing_info.get_routing_info_from_pre_vertex(vertex, part_id)
         if r_info is None:
             raise Exception(
                 f"Missing Routing information for {vertex}, {part_id}")
         entries = [(vertex, part_id, entry, r_info)]
         while __match(iterator, vertex, part_id, r_info, entry, routing_info):
-            (vertex, part_id), entry = iterator.next
+            (vertex, part_id), entry = iterator.pop()
             r_info = routing_info.get_routing_info_from_pre_vertex(
                 vertex, part_id)
             entries.append((vertex, part_id, entry, r_info))
@@ -71,9 +71,6 @@ def __create_routing_table(x, y, partitions_in_table, routing_info):
         for entry in __merged_keys_and_masks(entries, routing_info):
             table.add_multicast_routing_entry(entry)
 
-    for source_vertex, partition_id in partitions_in_table:
-        entry = partitions_in_table[source_vertex, partition_id]
-
     return table
 
 
@@ -82,7 +79,7 @@ def __match(iterator, vertex, part_id, r_info, entry, routing_info):
         return False
     if isinstance(vertex, ApplicationVertex):
         return False
-    (next_vertex, next_part_id), next_entry = iterator.peek
+    (next_vertex, next_part_id), next_entry = iterator.peek()
     if isinstance(next_vertex, ApplicationVertex):
         return False
     if part_id != next_part_id:
@@ -128,18 +125,6 @@ def __merged_keys_and_masks(entries, routing_info):
         yield from app_r_info.merge_machine_entries(entries)
 
 
-def __create_entry(key_and_mask, entry):
-    """
-    :param BaseKeyAndMask key_and_mask:
-    :param MulticastRoutingTableByPartitionEntry entry:
-    :rtype: MulticastRoutingEntry
-    """
-    return MulticastRoutingEntry(
-        routing_entry_key=key_and_mask.key_combo,
-        defaultable=entry.defaultable, mask=key_and_mask.mask,
-        link_ids=entry.link_ids, processor_ids=entry.processor_ids)
-
-
 class _IteratorWithNext(object):
 
     def __init__(self, iterable):
@@ -151,16 +136,14 @@ def __init__(self, iterable):
             self.__next = None
             self.__has_next = False
 
-    @property
     def peek(self):
         return self.__next
 
     @property
     def has_next(self):
         return self.__has_next
 
-    @property
-    def next(self):
+    def pop(self):
         if not self.__has_next:
             raise StopIteration
         nxt = self.__next

pacman/utilities/algorithm_utilities/routing_tree.py

@@ -47,7 +47,7 @@ def __init__(self, chip, label=None):
         :param tuple(int,int) chip:
             The chip the route is currently passing through.
         """
-        self.chip = chip
+        self._chip_x, self._chip_y = chip
         self._children = []
         self._label = label
 
@@ -63,10 +63,6 @@ def chip(self):
         """
         return (self._chip_x, self._chip_y)
 
-    @chip.setter
-    def chip(self, chip):
-        self._chip_x, self._chip_y = chip
-
     @property
     def children(self):
         """
@@ -91,22 +87,22 @@ def children(self):
           additional logic may be required to determine what core to target to\
           reach the vertex.
 
-        :rtype: iterable(RoutingTree or MachineVertex)
+        :rtype: iterable(tuple(int, RoutingTree or MachineVertex))
         """
         for child in self._children:
             yield child
 
     def append_child(self, child):
         """
         :param child:
-        :type child: RoutingTree or MachineVertex
+        :type child: tuple(int, RoutingTree or MachineVertex)
         """
         self._children.append(child)
 
     def remove_child(self, child):
         """
         :param child:
-        :type child: RoutingTree or MachineVertex
+        :type child: tuple(int, RoutingTree or MachineVertex)
         """
         self._children.remove(child)