From 5e6e2175145b731e1b17c8875cb5f86e3c3c1236 Mon Sep 17 00:00:00 2001 From: christian-B Date: Wed, 14 Oct 2020 15:02:40 +0100 Subject: [PATCH 1/9] all or nothing for machinevertex.appvertex --- pacman/model/graphs/machine/machine_graph.py | 36 +++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/pacman/model/graphs/machine/machine_graph.py b/pacman/model/graphs/machine/machine_graph.py index 6ae380edc..0aad146c7 100644 --- a/pacman/model/graphs/machine/machine_graph.py +++ b/pacman/model/graphs/machine/machine_graph.py @@ -15,6 +15,8 @@ from .machine_vertex import MachineVertex from .machine_edge import MachineEdge +from spinn_utilities.overrides import overrides +from pacman.exceptions import PacmanInvalidParameterException from pacman.model.graphs.graph import Graph from pacman.model.graphs import OutgoingEdgePartition @@ -23,7 +25,19 @@ class MachineGraph(Graph): """ A graph whose vertices can fit on the chips of a machine. """ - __slots__ = [] + __slots__ = [ + # Flags to say the application level is used so all machine vertices + # will have an application vertex + "_application_level_used", + ] + + MISSING_APP_VERTEX_ERROR_MESSAGE = ( + "The vertex does not have an app_vertex, " + "which is required when other app_vertices exist.") + + UNEXPECTED_APP_VERTEX_ERROR_MESSAGE = ( + "The vertex has an app_vertex, " + "which is not allowed when others not have app_vertices.") def __init__(self, label, application_graph=None): """ @@ -38,3 +52,23 @@ def __init__(self, label, application_graph=None): MachineVertex, MachineEdge, OutgoingEdgePartition, label) if application_graph: application_graph.forget_machine_graph() + # Check the first vertex added + self._application_level_used = None + else: + # Must be false as there is no App_graph + self._application_level_used = False + + @overrides(Graph.add_vertex) + def add_vertex(self, vertex): + super(MachineGraph, self).add_vertex(vertex) + if self._application_level_used is None: + self._application_level_used = vertex.app_vertex is not None + if self._application_level_used: + if not vertex.app_vertex: + raise PacmanInvalidParameterException( + "vertex", vertex, self.MISSING_APP_VERTEX_ERROR_MESSAGE) + else: + if vertex.app_vertex: + raise PacmanInvalidParameterException( + "vertex", vertex, self.UNEXPECTED_APP_VERTEX_ERROR_MESSAGE) + assert(vertex.app_vertex is None) From 0dc68fdce67324ffe31bedb932dfd09577609a12 Mon Sep 17 00:00:00 2001 From: christian-B Date: Thu, 15 Oct 2020 08:17:00 +0100 Subject: [PATCH 2/9] better way of checking app vertex --- pacman/model/graphs/machine/machine_graph.py | 12 +++--- .../test_machine_graph_model.py | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/pacman/model/graphs/machine/machine_graph.py b/pacman/model/graphs/machine/machine_graph.py index 0aad146c7..929f9e4cb 100644 --- a/pacman/model/graphs/machine/machine_graph.py +++ b/pacman/model/graphs/machine/machine_graph.py @@ -53,7 +53,7 @@ def __init__(self, label, application_graph=None): if application_graph: application_graph.forget_machine_graph() # Check the first vertex added - self._application_level_used = None + self._application_level_used = True else: # Must be false as there is no App_graph self._application_level_used = False @@ -61,12 +61,14 @@ def __init__(self, label, application_graph=None): @overrides(Graph.add_vertex) def add_vertex(self, vertex): super(MachineGraph, self).add_vertex(vertex) - if self._application_level_used is None: - self._application_level_used = vertex.app_vertex is not None if self._application_level_used: if not vertex.app_vertex: - raise PacmanInvalidParameterException( - "vertex", vertex, self.MISSING_APP_VERTEX_ERROR_MESSAGE) + if self.n_vertices == 1: + self._application_level_used = False + else: + raise PacmanInvalidParameterException( + "vertex", vertex, + self.MISSING_APP_VERTEX_ERROR_MESSAGE) else: if vertex.app_vertex: raise PacmanInvalidParameterException( diff --git a/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py b/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py index e6eddfb1c..8e22aed2b 100644 --- a/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py +++ b/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py @@ -14,10 +14,12 @@ # along with this program. If not, see . import unittest +from pacman.model.graphs.application import ApplicationGraph from pacman.model.graphs.machine import ( MachineEdge, MachineGraph, SimpleMachineVertex) from pacman.exceptions import ( PacmanAlreadyExistsException, PacmanInvalidParameterException) +from uinit_test_objects import SimpleTestVertex class TestMachineGraphModel(unittest.TestCase): @@ -113,6 +115,41 @@ def test_add_duplicate_edge(self): with self.assertRaises(PacmanAlreadyExistsException): graph.add_edges(edges, "bar") + def test_all_have_app_vertex(self): + app_graph = ApplicationGraph("Test") + graph = MachineGraph("foo", app_graph) + app1 = SimpleTestVertex(12, "app1") + mach1 = SimpleMachineVertex("mach1", app_vertex=app1) + mach2 = SimpleMachineVertex("mach2", app_vertex=app1) + mach3 = SimpleMachineVertex("mach3", app_vertex=None) + graph.add_vertices([mach1, mach2]) + with self.assertRaises(PacmanInvalidParameterException): + graph.add_vertex(mach3) + + def test_none_have_app_vertex(self): + app_graph = ApplicationGraph("Test") + graph = MachineGraph("foo", app_graph) + app1 = SimpleTestVertex(12, "app1") + mach1 = SimpleMachineVertex("mach1", app_vertex=None) + mach2 = SimpleMachineVertex("mach2", app_vertex=None) + mach3 = SimpleMachineVertex("mach3", app_vertex=app1) + graph.add_vertices([mach1, mach2]) + with self.assertRaises(PacmanInvalidParameterException): + graph.add_vertex(mach3) + + def test_no_app_graph_no_app_vertex(self): + graph = MachineGraph("foo") + app1 = SimpleTestVertex(12, "app1") + mach1 = SimpleMachineVertex("mach1", app_vertex=app1) + mach2 = SimpleMachineVertex("mach2", app_vertex=None) + mach3 = SimpleMachineVertex("mach3", app_vertex=app1) + with self.assertRaises(PacmanInvalidParameterException): + graph.add_vertex(mach1) + graph.add_vertex(mach2) + with self.assertRaises(PacmanInvalidParameterException): + graph.add_vertex(mach3) + + def test_add_edge_with_no_existing_pre_vertex_in_graph(self): """ test that adding a edge where the pre vertex has not been added From 1e1e29700eedecc6b4628b1fa7b58888f56b9f85 Mon Sep 17 00:00:00 2001 From: christian-B Date: Thu, 15 Oct 2020 09:31:46 +0100 Subject: [PATCH 3/9] flake8 --- .../model_tests/machine_graph_tests/test_machine_graph_model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py b/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py index 8e22aed2b..4af67b71e 100644 --- a/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py +++ b/unittests/model_tests/machine_graph_tests/test_machine_graph_model.py @@ -149,7 +149,6 @@ def test_no_app_graph_no_app_vertex(self): with self.assertRaises(PacmanInvalidParameterException): graph.add_vertex(mach3) - def test_add_edge_with_no_existing_pre_vertex_in_graph(self): """ test that adding a edge where the pre vertex has not been added From 018376708e06b9d320aa3dae7959198311e11b17 Mon Sep 17 00:00:00 2001 From: christian-B Date: Fri, 23 Oct 2020 10:13:36 +0100 Subject: [PATCH 4/9] cheaper check --- pacman/model/graphs/machine/machine_graph.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pacman/model/graphs/machine/machine_graph.py b/pacman/model/graphs/machine/machine_graph.py index 75035e0f1..0abbe77ec 100644 --- a/pacman/model/graphs/machine/machine_graph.py +++ b/pacman/model/graphs/machine/machine_graph.py @@ -62,7 +62,9 @@ def __init__(self, label, application_graph=None): def add_vertex(self, vertex): super(MachineGraph, self).add_vertex(vertex) if self._application_level_used: - if not vertex.app_vertex: + try: + vertex.app_vertex.remember_machine_vertex(vertex) + except AttributeError: if self.n_vertices == 1: self._application_level_used = False else: @@ -73,6 +75,3 @@ def add_vertex(self, vertex): if vertex.app_vertex: raise PacmanInvalidParameterException( "vertex", vertex, self.UNEXPECTED_APP_VERTEX_ERROR_MESSAGE) - assert(vertex.app_vertex is None) - if vertex.app_vertex: - vertex.app_vertex.remember_machine_vertex(vertex) From 7515a9fac709d7d3349034a5a103e0e15b4eec91 Mon Sep 17 00:00:00 2001 From: christian-B Date: Mon, 26 Oct 2020 16:26:50 +0000 Subject: [PATCH 5/9] refactor splitter_object to splitter --- .../graphs/application/application_vertex.py | 32 +++++++------- .../abstract_splitter_common.py | 6 +-- .../splitter_partitioner.py | 14 +++--- .../test_basic_partitioner.py | 22 +++++----- .../test_partition_and_place_partitioner.py | 44 +++++++++---------- ...artitioner_with_pre_allocated_resources.py | 10 ++--- 6 files changed, 64 insertions(+), 64 deletions(-) diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py index dbb71c62d..57b368865 100644 --- a/pacman/model/graphs/application/application_vertex.py +++ b/pacman/model/graphs/application/application_vertex.py @@ -36,9 +36,9 @@ class ApplicationVertex(AbstractVertex): "_machine_vertices", # the splitter object associated with this app vertex - "_splitter_object"] + "_splitter"] - SETTING_SPLITTER_OBJECT_ERROR_MSG = ( + SETTING_SPLITTER_ERROR_MSG = ( "The splitter object on {} has already been set, it cannot be " "reset. Please fix and try again. ") @@ -53,7 +53,7 @@ def __init__(self, label=None, constraints=None, :raise PacmanInvalidParameterException: If one of the constraints is not valid """ - self._splitter_object = None + self._splitter = None super(ApplicationVertex, self).__init__(label, constraints) self._machine_vertices = OrderedSet() @@ -69,32 +69,32 @@ def __repr__(self): self.label, self.constraints) @property - def splitter_object(self): + def splitter(self): """ :rtype: ~pacman.model.partitioner_interfaces.AbstractSplitterCommon """ - return self._splitter_object + return self._splitter - @splitter_object.setter - def splitter_object(self, new_value): + @splitter.setter + def splitter(self, new_value): """ sets the splitter object. Does not allow repeated settings. :param SplitterObjectCommon new_value: the new splitter object :rtype: None """ - if self._splitter_object == new_value: + if self._splitter == new_value: return - if self._splitter_object is not None: + if self._splitter is not None: raise PacmanConfigurationException( - self.SETTING_SPLITTER_OBJECT_ERROR_MSG.format(self._label)) - self._splitter_object = new_value - self._splitter_object.set_governed_app_vertex(self) + self.SETTING_SPLITTER_ERROR_MSG.format(self._label)) + self._splitter = new_value + self._splitter.set_governed_app_vertex(self) @overrides(AbstractVertex.add_constraint) def add_constraint(self, constraint): AbstractVertex.add_constraint(self, constraint) - if self._splitter_object is not None: - self._splitter_object.check_supported_constraints() + if self._splitter is not None: + self._splitter.check_supported_constraints() def remember_machine_vertex(self, machine_vertex): """ @@ -156,5 +156,5 @@ def forget_machine_vertices(self): vertex maps to. """ self._machine_vertices = OrderedSet() - if self._splitter_object is not None: - self._splitter_object.reset_called() + if self._splitter is not None: + self._splitter.reset_called() diff --git a/pacman/model/partitioner_interfaces/abstract_splitter_common.py b/pacman/model/partitioner_interfaces/abstract_splitter_common.py index 1671a7aa3..23a5adfb4 100644 --- a/pacman/model/partitioner_interfaces/abstract_splitter_common.py +++ b/pacman/model/partitioner_interfaces/abstract_splitter_common.py @@ -42,7 +42,7 @@ class AbstractSplitterCommon(object): "_is_fixed_atoms_per_core" ] - SETTING_SPLITTER_OBJECT_ERROR_MSG = ( + SETTING_SPLITTER_ERROR_MSG = ( "The app vertex {} is already governed by this " "{}. And so cannot govern app vertex {}." " Please fix and try again.") @@ -153,12 +153,12 @@ def set_governed_app_vertex(self, app_vertex): return if self._governed_app_vertex is not None: raise PacmanConfigurationException( - self.SETTING_SPLITTER_OBJECT_ERROR_MSG.format( + self.SETTING_SPLITTER_ERROR_MSG.format( self._governed_app_vertex, self._splitter_name, app_vertex)) self._governed_app_vertex = app_vertex self.check_supported_constraints() - app_vertex.splitter_object = self + app_vertex.splitter = self def check_supported_constraints(self): utility_calls.check_algorithm_can_support_constraints( diff --git a/pacman/operations/partition_algorithms/splitter_partitioner.py b/pacman/operations/partition_algorithms/splitter_partitioner.py index afb5862b0..dea4abd6c 100644 --- a/pacman/operations/partition_algorithms/splitter_partitioner.py +++ b/pacman/operations/partition_algorithms/splitter_partitioner.py @@ -104,7 +104,7 @@ def __call__( # Partition one vertex at a time for vertex in progress.over(vertices): - vertex.splitter_object.split(resource_tracker, machine_graph) + vertex.splitter.split(resource_tracker, machine_graph) # process edges self.__process_machine_edges( @@ -125,9 +125,9 @@ def order_vertices_for_dependent_splitters(vertices): """ dependent_vertices = OrderedDict() for vertex in vertices: - if isinstance(vertex.splitter_object, AbstractDependentSplitter): + if isinstance(vertex.splitter, AbstractDependentSplitter): other_app_vertex = ( - vertex.splitter_object.other_splitter.governed_app_vertex) + vertex.splitter.other_splitter.governed_app_vertex) dependent_vertices[other_app_vertex] = vertex for main_vertex in dependent_vertices.keys(): @@ -183,9 +183,9 @@ def __set_same_max_atoms_to_splitters(self, app_graph): vertex.n_atoms, fixed_n_atoms)) for other_vertex in partition_together_vertices: - other_vertex.splitter_object.set_max_atoms_per_core( + other_vertex.splitter.set_max_atoms_per_core( max_atoms_per_core, fixed_n_atoms is not None) - vertex.splitter_object.set_max_atoms_per_core( + vertex.splitter.set_max_atoms_per_core( max_atoms_per_core, fixed_n_atoms is not None) def __setup_objects( @@ -268,13 +268,13 @@ def __process_machine_edges( # go through each edge for app_edge in app_outgoing_edge_partition.edges: src_vertices_edge_type_map = ( - app_edge.pre_vertex.splitter_object.get_pre_vertices( + app_edge.pre_vertex.splitter.get_pre_vertices( app_edge, app_outgoing_edge_partition)) # go through each pre vertices for src_machine_vertex in src_vertices_edge_type_map: dest_vertices_edge_type_map = ( - app_edge.post_vertex.splitter_object.get_post_vertices( + app_edge.post_vertex.splitter.get_post_vertices( app_edge, app_outgoing_edge_partition, src_machine_vertex)) diff --git a/unittests/operations_tests/partition_algorithms_tests/test_basic_partitioner.py b/unittests/operations_tests/partition_algorithms_tests/test_basic_partitioner.py index 323559c24..c5d0892cf 100644 --- a/unittests/operations_tests/partition_algorithms_tests/test_basic_partitioner.py +++ b/unittests/operations_tests/partition_algorithms_tests/test_basic_partitioner.py @@ -45,11 +45,11 @@ def setup(self): setup for all basic partitioner tests """ self.vert1 = SimpleTestVertex(10, "New AbstractConstrainedVertex 1") - self.vert1.splitter_object = SplitterSliceLegacy() + self.vert1.splitter = SplitterSliceLegacy() self.vert2 = SimpleTestVertex(5, "New AbstractConstrainedVertex 2") - self.vert2.splitter_object = SplitterSliceLegacy() + self.vert2.splitter = SplitterSliceLegacy() self.vert3 = SimpleTestVertex(3, "New AbstractConstrainedVertex 3") - self.vert3.splitter_object = SplitterSliceLegacy() + self.vert3.splitter = SplitterSliceLegacy() self.edge1 = ApplicationEdge( self.vert1, self.vert2, label="First edge") self.edge2 = ApplicationEdge( @@ -121,7 +121,7 @@ def test_partition_on_large_vertex_than_has_to_be_split(self): """ self.setup() large_vertex = SimpleTestVertex(300, "Large vertex") - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) self.assertEqual(large_vertex._model_based_max_atoms_per_core, 256) @@ -135,7 +135,7 @@ def test_partition_on_very_large_vertex_than_has_to_be_split(self): """ self.setup() large_vertex = SimpleTestVertex(500, "Large vertex") - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.assertEqual(large_vertex._model_based_max_atoms_per_core, 256) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) @@ -150,7 +150,7 @@ def test_partition_on_target_size_vertex_than_has_to_be_split(self): self.setup() large_vertex = SimpleTestVertex(1000, "Large vertex") large_vertex.add_constraint(MaxVertexAtomsConstraint(10)) - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) graph, _ = self.bp(self.graph, self.machine, 3000) @@ -190,7 +190,7 @@ def test_partition_with_barely_sufficient_space(self): n_neurons = 17 * 5 * 5 singular_vertex = SimpleTestVertex(n_neurons, "Large vertex", max_atoms_per_core=1) - singular_vertex.splitter_object = SplitterSliceLegacy() + singular_vertex.splitter = SplitterSliceLegacy() self.assertEqual(singular_vertex._model_based_max_atoms_per_core, 1) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(singular_vertex) @@ -232,7 +232,7 @@ def test_partition_with_insufficient_space(self): self.machine = machine_from_chips(chips) large_vertex = SimpleTestVertex(3000, "Large vertex", max_atoms_per_core=1) - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.assertEqual(large_vertex._model_based_max_atoms_per_core, 1) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) @@ -317,7 +317,7 @@ def test_partition_with_unsupported_constraints(self): constrained_vertex.add_constraint( NewPartitionerConstraint("Mock constraint")) with self.assertRaises(PacmanInvalidParameterException): - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() def test_partition_with_empty_graph(self): """ @@ -348,7 +348,7 @@ def test_partition_with_fixed_atom_constraints(self): vertex = SimpleTestVertex( sdram_per_chip * machine.n_chips, max_atoms_per_core=2, constraints=[FixedVertexAtomsConstraint(2)]) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() app_graph = ApplicationGraph("Test") app_graph.add_vertex(vertex) @@ -376,7 +376,7 @@ def test_partition_with_fixed_atom_constraints_at_limit(self): vertex = SimpleTestVertex( sdram_per_chip * 2, max_atoms_per_core=sdram_per_chip, constraints=[FixedVertexAtomsConstraint(sdram_per_chip // 2)]) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() app_graph = ApplicationGraph("Test") app_graph.add_vertex(vertex) diff --git a/unittests/operations_tests/partition_algorithms_tests/test_partition_and_place_partitioner.py b/unittests/operations_tests/partition_algorithms_tests/test_partition_and_place_partitioner.py index ab50d8549..03fe0db14 100644 --- a/unittests/operations_tests/partition_algorithms_tests/test_partition_and_place_partitioner.py +++ b/unittests/operations_tests/partition_algorithms_tests/test_partition_and_place_partitioner.py @@ -43,11 +43,11 @@ def setup(self): """setup for all basic partitioner tests """ self.vert1 = SimpleTestVertex(10, "New AbstractConstrainedVertex 1") - self.vert1.splitter_object = SplitterSliceLegacy() + self.vert1.splitter = SplitterSliceLegacy() self.vert2 = SimpleTestVertex(5, "New AbstractConstrainedVertex 2") - self.vert2.splitter_object = SplitterSliceLegacy() + self.vert2.splitter = SplitterSliceLegacy() self.vert3 = SimpleTestVertex(3, "New AbstractConstrainedVertex 3") - self.vert3.splitter_object = SplitterSliceLegacy() + self.vert3.splitter = SplitterSliceLegacy() self.edge1 = ApplicationEdge( self.vert1, self.vert2, label="First edge") self.edge2 = ApplicationEdge( @@ -121,7 +121,7 @@ def test_partition_on_large_vertex_than_has_to_be_split(self): """ self.setup() large_vertex = SimpleTestVertex(300, "Large vertex") - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) graph, _ = self.bp( @@ -137,7 +137,7 @@ def test_partition_on_very_large_vertex_than_has_to_be_split(self): """ self.setup() large_vertex = SimpleTestVertex(500, "Large vertex") - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.assertEqual(large_vertex._model_based_max_atoms_per_core, 256) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) @@ -154,7 +154,7 @@ def test_partition_on_target_size_vertex_than_has_to_be_split(self): self.setup() large_vertex = SimpleTestVertex(1000, "Large vertex") large_vertex.add_constraint(MaxVertexAtomsConstraint(10)) - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) graph, _ = self.bp( @@ -197,7 +197,7 @@ def test_partition_with_barely_sufficient_space(self): n_neurons = 17 * 5 * 5 singular_vertex = SimpleTestVertex(n_neurons, "Large vertex", max_atoms_per_core=1) - singular_vertex.splitter_object = SplitterSliceLegacy() + singular_vertex.splitter = SplitterSliceLegacy() self.assertEqual(singular_vertex._model_based_max_atoms_per_core, 1) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(singular_vertex) @@ -241,7 +241,7 @@ def test_partition_with_insufficient_space(self): self.machine = machine_from_chips(chips) large_vertex = SimpleTestVertex(3000, "Large vertex", max_atoms_per_core=1) - large_vertex.splitter_object = SplitterSliceLegacy() + large_vertex.splitter = SplitterSliceLegacy() self.assertEqual(large_vertex._model_based_max_atoms_per_core, 1) self.graph = ApplicationGraph("Graph with large vertex") self.graph.add_vertex(large_vertex) @@ -329,7 +329,7 @@ def test_partition_with_unsupported_constraints(self): constrained_vertex.add_constraint( NewPartitionerConstraint("Mock constraint")) with self.assertRaises(PacmanInvalidParameterException): - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() def test_partition_with_empty_graph(self): """test that the partitioner can work with an empty graph @@ -350,7 +350,7 @@ def test_operation_with_same_size_as_vertex_constraint(self): constrained_vertex = SimpleTestVertex(5, "Constrained") constrained_vertex.add_constraint( SameAtomsAsVertexConstraint(self.vert2)) - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertex(constrained_vertex) graph, _ = self.bp( self.graph, self.machine, plan_n_time_steps=100, @@ -368,8 +368,8 @@ def test_operation_with_same_size_as_vertex_constraint_large_vertices( new_large_vertex = SimpleTestVertex(300, "Non constrained") constrained_vertex.add_constraint( SameAtomsAsVertexConstraint(new_large_vertex)) - new_large_vertex.splitter_object = SplitterSliceLegacy() - constrained_vertex.splitter_object = SplitterSliceLegacy() + new_large_vertex.splitter = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertices([new_large_vertex, constrained_vertex]) graph, _ = self.bp( self.graph, self.machine, plan_n_time_steps=100, @@ -387,8 +387,8 @@ def test_operation_same_size_as_vertex_constraint_different_order(self): new_large_vertex = SimpleTestVertex(300, "Non constrained") constrained_vertex.add_constraint( SameAtomsAsVertexConstraint(new_large_vertex)) - constrained_vertex.splitter_object = SplitterSliceLegacy() - new_large_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() + new_large_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertices([constrained_vertex, new_large_vertex]) graph, _ = self.bp( self.graph, self.machine, plan_n_time_steps=100, @@ -405,7 +405,7 @@ def test_operation_with_same_size_as_vertex_constraint_exception(self): constrained_vertex = SimpleTestVertex(100, "Constrained") constrained_vertex.add_constraint( SameAtomsAsVertexConstraint(self.vert2)) - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertex(constrained_vertex) partitioner = SplitterPartitioner() self.assertRaises(PacmanPartitionException, partitioner, @@ -418,13 +418,13 @@ def test_operation_with_same_size_as_vertex_constraint_chain(self): """ graph = ApplicationGraph("Test") vertex_1 = SimpleTestVertex(10, "Vertex_1", 5) - vertex_1.splitter_object = SplitterSliceLegacy() + vertex_1.splitter = SplitterSliceLegacy() vertex_2 = SimpleTestVertex(10, "Vertex_2", 4) vertex_3 = SimpleTestVertex(10, "Vertex_3", 2) vertex_3.add_constraint(SameAtomsAsVertexConstraint(vertex_2)) vertex_2.add_constraint(SameAtomsAsVertexConstraint(vertex_1)) - vertex_2.splitter_object = SplitterSliceLegacy() - vertex_3.splitter_object = SplitterSliceLegacy() + vertex_2.splitter = SplitterSliceLegacy() + vertex_3.splitter = SplitterSliceLegacy() graph.add_vertices([vertex_1, vertex_2, vertex_3]) machine = virtual_machine(width=2, height=2) partitioner = SplitterPartitioner() @@ -438,10 +438,10 @@ def test_operation_with_same_size_as_vertex_constraint_chain(self): def test_partitioning_with_2_massive_pops(self): self.setup() constrained_vertex = SimpleTestVertex(16000, "Constrained") - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertex(constrained_vertex) constrained_vertex = SimpleTestVertex(16000, "Constrained") - constrained_vertex.splitter_object = SplitterSliceLegacy() + constrained_vertex.splitter = SplitterSliceLegacy() self.graph.add_vertex(constrained_vertex) partitioner = SplitterPartitioner() partitioner( @@ -467,7 +467,7 @@ def test_partition_with_fixed_atom_constraints(self): vertex = SimpleTestVertex( sdram_per_chip * machine.n_chips, max_atoms_per_core=2, constraints=[FixedVertexAtomsConstraint(2)]) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() app_graph = ApplicationGraph("Test") app_graph.add_vertex(vertex) @@ -495,7 +495,7 @@ def test_partition_with_fixed_atom_constraints_at_limit(self): vertex = SimpleTestVertex( sdram_per_chip * 2, max_atoms_per_core=sdram_per_chip, constraints=[FixedVertexAtomsConstraint(sdram_per_chip // 2)]) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() app_graph = ApplicationGraph("Test") app_graph.add_vertex(vertex) diff --git a/unittests/operations_tests/partition_algorithms_tests/test_partitioner_with_pre_allocated_resources.py b/unittests/operations_tests/partition_algorithms_tests/test_partitioner_with_pre_allocated_resources.py index 8181aa74d..0f77f10ca 100644 --- a/unittests/operations_tests/partition_algorithms_tests/test_partitioner_with_pre_allocated_resources.py +++ b/unittests/operations_tests/partition_algorithms_tests/test_partitioner_with_pre_allocated_resources.py @@ -46,7 +46,7 @@ def test_1_chip_over_pre_allocated(self): vertex = SimpleTestVertex( constraints=[ChipAndCoreConstraint(x=0, y=0)], n_atoms=1) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() graph.add_vertex(vertex) # add pre-allocated resources for cores on 0,0 @@ -72,7 +72,7 @@ def test_1_chip_under_pre_allocated(self): vertex = SimpleTestVertex( constraints=[ChipAndCoreConstraint(x=0, y=0)], n_atoms=1) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() graph.add_vertex(vertex) # add pre-allocated resources for cores on 0,0 @@ -97,7 +97,7 @@ def test_1_chip_pre_allocated_same_core(self): vertex = SimpleTestVertex( constraints=[ChipAndCoreConstraint(x=0, y=0, p=p)], n_atoms=1) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() graph.add_vertex(vertex) # add pre-allocated resources for cores on 0,0 @@ -129,7 +129,7 @@ def test_1_chip_pre_allocated_too_much_sdram(self): constraints=[ChipAndCoreConstraint(x=0, y=0)], n_atoms=1, fixed_sdram_value=eight_meg) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() graph.add_vertex(vertex) # add pre-allocated resources for cores on 0,0 @@ -163,7 +163,7 @@ def test_1_chip_no_pre_allocated_too_much_sdram(self): constraints=[ChipAndCoreConstraint(x=0, y=0)], n_atoms=1, fixed_sdram_value=eight_meg) - vertex.splitter_object = SplitterSliceLegacy() + vertex.splitter = SplitterSliceLegacy() graph.add_vertex(vertex) # add pre-allocated resources for cores on 0,0 From f6a335777a9ac3a4da7f422fb33cdaefaf4acfe9 Mon Sep 17 00:00:00 2001 From: christian-B Date: Mon, 26 Oct 2020 17:26:44 +0000 Subject: [PATCH 6/9] added splitter to application_vertex init --- .../graphs/application/application_vertex.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py index 57b368865..64647eb4f 100644 --- a/pacman/model/graphs/application/application_vertex.py +++ b/pacman/model/graphs/application/application_vertex.py @@ -43,17 +43,19 @@ class ApplicationVertex(AbstractVertex): "reset. Please fix and try again. ") def __init__(self, label=None, constraints=None, - max_atoms_per_core=sys.maxsize): + max_atoms_per_core=sys.maxsize, splitter=None): """ :param str label: The optional name of the vertex. :param iterable(AbstractConstraint) constraints: The optional initial constraints of the vertex. :param int max_atoms_per_core: The max number of atoms that can be placed on a core, used in partitioning. + :param splitter: The splitter object needed for this vertex. + Leave as None to delegate the choice of splitter to the selector. + :type splitter None or AbstractSplitterCommon :raise PacmanInvalidParameterException: If one of the constraints is not valid """ - self._splitter = None super(ApplicationVertex, self).__init__(label, constraints) self._machine_vertices = OrderedSet() @@ -61,6 +63,11 @@ def __init__(self, label=None, constraints=None, self.add_constraint( MaxVertexAtomsConstraint(max_atoms_per_core)) + # Need to set to None temporarily as setter check previous value + self._splitter = None + # Use setter as there is extra work to do + self.splitter = splitter + def __str__(self): return self.label @@ -82,11 +89,6 @@ def splitter(self, new_value): :param SplitterObjectCommon new_value: the new splitter object :rtype: None """ - if self._splitter == new_value: - return - if self._splitter is not None: - raise PacmanConfigurationException( - self.SETTING_SPLITTER_ERROR_MSG.format(self._label)) self._splitter = new_value self._splitter.set_governed_app_vertex(self) From de85f1a33278727ee0c7efbb46cba903a7550393 Mon Sep 17 00:00:00 2001 From: christian-B Date: Tue, 27 Oct 2020 06:14:05 +0000 Subject: [PATCH 7/9] fixed splitter setter --- .../graphs/application/application_vertex.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py index 64647eb4f..d11c7c1de 100644 --- a/pacman/model/graphs/application/application_vertex.py +++ b/pacman/model/graphs/application/application_vertex.py @@ -56,17 +56,18 @@ def __init__(self, label=None, constraints=None, :raise PacmanInvalidParameterException: If one of the constraints is not valid """ + # Need to set to None temporarily as add_constraint checks splitter + self._splitter = None super(ApplicationVertex, self).__init__(label, constraints) self._machine_vertices = OrderedSet() + # Use setter as there is extra work to do + self.splitter = splitter + # add a constraint for max partitioning self.add_constraint( MaxVertexAtomsConstraint(max_atoms_per_core)) - # Need to set to None temporarily as setter check previous value - self._splitter = None - # Use setter as there is extra work to do - self.splitter = splitter def __str__(self): return self.label @@ -89,8 +90,14 @@ def splitter(self, new_value): :param SplitterObjectCommon new_value: the new splitter object :rtype: None """ + if self._splitter == new_value: + return + if self._splitter is not None: + raise PacmanConfigurationException( + self.SETTING_SPLITTER_OBJECT_ERROR_MSG.format(self._label)) self._splitter = new_value self._splitter.set_governed_app_vertex(self) + self._splitter.check_supported_constraints() @overrides(AbstractVertex.add_constraint) def add_constraint(self, constraint): From 9ea961b938ffa3cc301e0a7532a10572f74f3cb3 Mon Sep 17 00:00:00 2001 From: christian-B Date: Tue, 27 Oct 2020 06:37:19 +0000 Subject: [PATCH 8/9] flake8 --- pacman/model/graphs/application/application_vertex.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py index d11c7c1de..e98acd8c6 100644 --- a/pacman/model/graphs/application/application_vertex.py +++ b/pacman/model/graphs/application/application_vertex.py @@ -68,7 +68,6 @@ def __init__(self, label=None, constraints=None, self.add_constraint( MaxVertexAtomsConstraint(max_atoms_per_core)) - def __str__(self): return self.label From c3956f689b1f895310c19f97eae5f72436013699 Mon Sep 17 00:00:00 2001 From: christian-B Date: Tue, 27 Oct 2020 07:06:31 +0000 Subject: [PATCH 9/9] refactor splitter_object to splitter --- pacman/model/graphs/application/application_vertex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py index e98acd8c6..62c632c46 100644 --- a/pacman/model/graphs/application/application_vertex.py +++ b/pacman/model/graphs/application/application_vertex.py @@ -93,7 +93,7 @@ def splitter(self, new_value): return if self._splitter is not None: raise PacmanConfigurationException( - self.SETTING_SPLITTER_OBJECT_ERROR_MSG.format(self._label)) + self.SETTING_SPLITTER_ERROR_MSG.format(self._label)) self._splitter = new_value self._splitter.set_governed_app_vertex(self) self._splitter.check_supported_constraints()