From 2d06d33b29a791a7f5f9c93b5aeae9a916fc9c60 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 09:49:34 +0100 Subject: [PATCH 01/16] Ensure that routing information is extracted correctly (or warn) --- pacman/model/routing_info/routing_info.py | 116 ++++++++++++++++++++-- 1 file changed, 105 insertions(+), 11 deletions(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 1edf268d6..55624af89 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. from __future__ import annotations -from typing import Dict, Iterator, Optional, Tuple, TYPE_CHECKING +from collections import defaultdict +from deprecated import deprecated +from typing import Dict, Iterator, Optional, Iterable, TYPE_CHECKING from pacman.exceptions import PacmanAlreadyExistsException if TYPE_CHECKING: from .vertex_routing_info import VertexRoutingInfo @@ -29,8 +31,8 @@ class RoutingInfo(object): def __init__(self) -> None: # Partition information indexed by edge pre-vertex and partition ID # name - self._info: Dict[ - Tuple[AbstractVertex, str], VertexRoutingInfo] = dict() + self._info: Dict[AbstractVertex, + Dict[str, VertexRoutingInfo]] = defaultdict(dict) def add_routing_info(self, info: VertexRoutingInfo): """ @@ -41,26 +43,58 @@ def add_routing_info(self, info: VertexRoutingInfo): :raise PacmanAlreadyExistsException: If the partition is already in the set of edges """ - key = (info.vertex, info.partition_id) - if key in self._info: + if (info.vertex in self._info and + info.partition_id in self._info[info.vertex]): raise PacmanAlreadyExistsException( "Routing information", str(info)) - self._info[key] = info + self._info[info.vertex][info.partition_id] = info + @deprecated(reason="This method is unsafe, since it doesn't determine " + "whether the info is missing because there is no " + "outgoing edge, or if the outgoing edge is in another " + "partition and the name is wrong. " + "Use a combination of " + "get_safe_routing_info_from_pre_vertex, " + "get_partitions_outgoing_from_vertex, " + "or get_single_routing_info_from_pre_vertex") def get_routing_info_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> Optional[VertexRoutingInfo]: """ Get routing information for a given partition_id from a vertex. + :param AbstractVertex vertex: The vertex to search for + :param str partition_id: + The ID of the partition for which to get the routing information + :rtype: VertexRoutingInfo or None + """ + return self._info[vertex].get(partition_id) + + def get_safe_routing_info_from_pre_vertex( + self, vertex: AbstractVertex, + partition_id: str) -> VertexRoutingInfo: + """ + Get routing information for a given partition_id from a vertex. + :param AbstractVertex vertex: The vertex to search for :param str partition_id: The ID of the partition for which to get the routing information :rtype: VertexRoutingInfo + :raise KeyError: + If the vertex/partition_id combination is not in the routing + information """ - return self._info.get((vertex, partition_id)) + return self._info[vertex][partition_id] + @deprecated(reason="This method is unsafe, since it doesn't determine " + "whether the info is missing because there is no " + "outgoing edge, or if the outgoing edge is in another " + "partition and the name is wrong. " + "Use a combination of " + "get_safe_first_key_from_pre_vertex, " + "get_partitions_outgoing_from_vertex, " + "or get_single_first_key_from_pre_vertex") def get_first_key_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> Optional[int]: """ @@ -70,12 +104,72 @@ def get_first_key_from_pre_vertex( :param str partition_id: The ID of the partition for which to get the routing information :return: The routing key of the partition + :rtype: int or None + """ + if vertex not in self._info: + return None + info = self._info[vertex] + if partition_id not in info: + return None + return info[partition_id].key + + def get_safe_first_key_from_pre_vertex( + self, vertex: AbstractVertex, partition_id: str) -> int: + """ + Get the first key for the partition starting at a vertex. + + :param AbstractVertex vertex: The vertex which the partition starts at + :param str partition_id: + The ID of the partition for which to get the routing information + :return: The routing key of the partition + :rtype: int + :raise KeyError: + If the vertex/partition_id combination is not in the routing + information + """ + return self._info[vertex][partition_id].key + + def get_partitions_outgoing_from_vertex( + self, vertex: AbstractVertex) -> Iterable[str]: + """ + Get the outgoing partitions from a vertex. + + :param AbstractVertex vertex: The vertex to search for + """ + return self._info[vertex].keys() + + def get_single_routing_info_from_pre_vertex( + self, vertex: AbstractVertex) -> Optional[VertexRoutingInfo]: + """ + Get routing information for a given vertex. Fails if the vertex has + more than one outgoing partition. + + :param AbstractVertex vertex: The vertex to search for + :rtype: VertexRoutingInfo or None + :raise KeyError: If the vertex has more than one outgoing partition + """ + if vertex not in self._info: + return None + info = self._info[vertex] + if len(info) != 1: + raise KeyError( + f"Vertex {vertex} has more than one outgoing partition") + return next(iter(info.values())) + + def get_single_first_key_from_pre_vertex( + self, vertex: AbstractVertex) -> int: + """ + Get the first key for the partition starting at a vertex. Fails if + the vertex has more than one outgoing partition. + + :param AbstractVertex vertex: The vertex which the partition starts at :rtype: int + :raise KeyError: If the vertex has more than one outgoing partition """ - key = (vertex, partition_id) - if key in self._info: - return self._info[key].key - return None + info = self.get_single_routing_info_from_pre_vertex(vertex) + if info is None: + return None + return info.key def __iter__(self) -> Iterator[VertexRoutingInfo]: """ From bdf2a8d713d5d673ffc1c46f800b7fb69e8ef3a3 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 09:58:23 +0100 Subject: [PATCH 02/16] Fix and test --- pacman/model/routing_info/routing_info.py | 4 +- .../routing_info_tests/test_routing_info.py | 57 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 55624af89..08a1b31b4 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -177,4 +177,6 @@ def __iter__(self) -> Iterator[VertexRoutingInfo]: :return: a iterator of routing information """ - return iter(self._info.values()) + for vertex_info in self._info.values(): + for info in vertex_info.values(): + yield info diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 0b434e0e1..9f85e6a72 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -28,7 +28,7 @@ class TestRoutingInfo(unittest.TestCase): def setUp(self): unittest_setup() - def test_routing_info(self): + def test_routing_info_deprecated(self): pre_vertex = SimpleMachineVertex(ConstantSDRAM(0)) key = 12345 info = MachineVertexRoutingInfo( @@ -73,6 +73,61 @@ def test_routing_info(self): assert routing_info.get_routing_info_from_pre_vertex( pre_vertex, "Test2").get_keys().tolist() == [key] + def test_routing_info(self): + pre_vertex = SimpleMachineVertex(ConstantSDRAM(0)) + key = 12345 + info = MachineVertexRoutingInfo( + BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) + routing_info = RoutingInfo() + routing_info.add_routing_info(info) + + with self.assertRaises(PacmanAlreadyExistsException): + routing_info.add_routing_info(info) + + assert routing_info.get_safe_routing_info_from_pre_vertex( + pre_vertex, "Test") == info + with self.assertRaises(KeyError): + routing_info.get_safe_routing_info_from_pre_vertex( + None, "Test") + with self.assertRaises(KeyError): + routing_info.get_safe_routing_info_from_pre_vertex( + pre_vertex, "None") + + assert routing_info.get_safe_first_key_from_pre_vertex( + pre_vertex, "Test") == key + with self.assertRaises(KeyError): + routing_info.get_safe_first_key_from_pre_vertex( + None, "Test") + with self.assertRaises(KeyError): + routing_info.get_safe_first_key_from_pre_vertex( + pre_vertex, "None") + + assert list(routing_info.get_partitions_outgoing_from_vertex( + pre_vertex)) == ["Test"] + assert list(routing_info.get_partitions_outgoing_from_vertex( + None)) == [] + + assert next(iter(routing_info)) == info + + info2 = MachineVertexRoutingInfo( + BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) + + with self.assertRaises(PacmanAlreadyExistsException): + routing_info.add_routing_info(info2) + assert info != info2 + + info3 = MachineVertexRoutingInfo( + BaseKeyAndMask(key, FULL_MASK), "Test2", pre_vertex, 0) + routing_info.add_routing_info(info3) + assert info != info3 + assert routing_info.get_safe_routing_info_from_pre_vertex( + pre_vertex, "Test2") !=\ + routing_info.get_safe_routing_info_from_pre_vertex( + pre_vertex, "Test") + assert routing_info.get_safe_routing_info_from_pre_vertex( + pre_vertex, "Test2").get_keys().tolist() == [key] + + def test_base_key_and_mask(self): with self.assertRaises(PacmanConfigurationException): BaseKeyAndMask(0xF0, 0x40) From 28cb7bc2589adabc8f832437ea89026116f663d6 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 09:59:38 +0100 Subject: [PATCH 03/16] pylint --- pacman/model/routing_info/routing_info.py | 2 +- setup.cfg | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 08a1b31b4..5a55644d9 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -13,8 +13,8 @@ # limitations under the License. from __future__ import annotations from collections import defaultdict -from deprecated import deprecated from typing import Dict, Iterator, Optional, Iterable, TYPE_CHECKING +from deprecated import deprecated from pacman.exceptions import PacmanAlreadyExistsException if TYPE_CHECKING: from .vertex_routing_info import VertexRoutingInfo diff --git a/setup.cfg b/setup.cfg index 6ca755e52..e50414fc6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -50,6 +50,7 @@ include_package_data = True install_requires = jsonschema typing_extensions + deprecated SpiNNUtilities == 1!7.3.1 SpiNNMachine == 1!7.3.1 From 02c9d1ca9ae023d9c2cd6d80103e1c7ad0fc065e Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 10:01:13 +0100 Subject: [PATCH 04/16] Flake8 --- unittests/model_tests/routing_info_tests/test_routing_info.py | 1 - 1 file changed, 1 deletion(-) diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 9f85e6a72..a99789bd6 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -127,7 +127,6 @@ def test_routing_info(self): assert routing_info.get_safe_routing_info_from_pre_vertex( pre_vertex, "Test2").get_keys().tolist() == [key] - def test_base_key_and_mask(self): with self.assertRaises(PacmanConfigurationException): BaseKeyAndMask(0xF0, 0x40) From 21656ebc78a7d19a992b7ba3dd0d34e246c94218 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 10:18:58 +0100 Subject: [PATCH 05/16] Fix type and install types --- pacman/model/routing_info/routing_info.py | 4 ++-- setup.cfg | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 5a55644d9..aeab610e3 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -157,13 +157,13 @@ def get_single_routing_info_from_pre_vertex( return next(iter(info.values())) def get_single_first_key_from_pre_vertex( - self, vertex: AbstractVertex) -> int: + self, vertex: AbstractVertex) -> Optional[int]: """ Get the first key for the partition starting at a vertex. Fails if the vertex has more than one outgoing partition. :param AbstractVertex vertex: The vertex which the partition starts at - :rtype: int + :rtype: int or None :raise KeyError: If the vertex has more than one outgoing partition """ info = self.get_single_routing_info_from_pre_vertex(vertex) diff --git a/setup.cfg b/setup.cfg index e50414fc6..0eddbcdfb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -74,6 +74,7 @@ test = pylint testfixtures types-jsonschema + types-Deprecated # How to make # [Reports] # draw_placements=True From 107075a65d64a3b372b580022c05ffa89241094d Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 10:51:42 +0100 Subject: [PATCH 06/16] Fix other uses of the functions --- .../valid_routes_checker.py | 9 ++++----- .../basic_routing_table_generator.py | 5 +---- .../merged_routing_table_generator.py | 15 +++++---------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py b/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py index b7ee5cac3..0c4e9b591 100644 --- a/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py +++ b/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py @@ -118,14 +118,13 @@ def validate_routes(routing_tables: MulticastRoutingTables): if isinstance(m_vertex, AbstractVirtual): continue placement = PacmanDataView.get_placement_of_vertex(m_vertex) - r_info = routing_infos.get_routing_info_from_pre_vertex( + r_info = routing_infos.get_safe_routing_info_from_pre_vertex( m_vertex, partition.identifier) # search for these destinations - if r_info: - _search_route( - placement, destinations[m_vertex], r_info.key_and_mask, - routing_tables, m_vertex.vertex_slice.n_atoms) + _search_route( + placement, destinations[m_vertex], r_info.key_and_mask, + routing_tables, m_vertex.vertex_slice.n_atoms) def _search_route( diff --git a/pacman/operations/routing_table_generators/basic_routing_table_generator.py b/pacman/operations/routing_table_generators/basic_routing_table_generator.py index 2f3f41a57..524794f81 100644 --- a/pacman/operations/routing_table_generators/basic_routing_table_generator.py +++ b/pacman/operations/routing_table_generators/basic_routing_table_generator.py @@ -63,11 +63,8 @@ def __create_routing_table( sources_by_key_mask: Dict[BaseKeyAndMask, Tuple[AbstractVertex, str]] = dict() for source_vertex, partition_id in partitions_in_table: - r_info = routing_infos.get_routing_info_from_pre_vertex( + r_info = routing_infos.get_safe_routing_info_from_pre_vertex( source_vertex, partition_id) - # Should be there; skip if not - if r_info is None: - continue entry = partitions_in_table[source_vertex, partition_id] if r_info.key_and_mask in sources_by_key_mask: if (sources_by_key_mask[r_info.key_and_mask] diff --git a/pacman/operations/routing_table_generators/merged_routing_table_generator.py b/pacman/operations/routing_table_generators/merged_routing_table_generator.py index 204bc608e..863ce8a7f 100644 --- a/pacman/operations/routing_table_generators/merged_routing_table_generator.py +++ b/pacman/operations/routing_table_generators/merged_routing_table_generator.py @@ -70,10 +70,8 @@ def __create_routing_table( iterator = _IteratorWithNext(partitions_in_table.items()) while iterator.has_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 PacmanRoutingException( - f"Missing Routing information for {vertex}, {part_id}") + r_info = routing_info.get_safe_routing_info_from_pre_vertex( + vertex, part_id) if r_info.key_and_mask in sources_by_key_mask: if (sources_by_key_mask[r_info.key_and_mask] != (vertex, part_id)): @@ -102,7 +100,7 @@ def __create_routing_table( continue # This has to be AppVertexRoutingInfo! - app_r_info = routing_info.get_routing_info_from_pre_vertex( + app_r_info = routing_info.get_safe_routing_info_from_pre_vertex( vertex.app_vertex, part_id) assert isinstance(app_r_info, AppVertexRoutingInfo) @@ -112,7 +110,7 @@ def __create_routing_table( while __match(iterator, vertex, part_id, r_info, entry, routing_info, app_r_info): (vertex, part_id), entry = iterator.pop() - r_info = routing_info.get_routing_info_from_pre_vertex( + r_info = routing_info.get_safe_routing_info_from_pre_vertex( vertex, part_id) if r_info is not None: assert isinstance(r_info, MachineVertexRoutingInfo) @@ -139,11 +137,8 @@ def __match( return False if __mask_has_holes(r_info.mask): return False - next_r_info = routing_info.get_routing_info_from_pre_vertex( + next_r_info = routing_info.get_safe_routing_info_from_pre_vertex( next_vertex, next_part_id) - if next_r_info is None: - raise KeyError( - f"No routing info found for {next_vertex}, {next_part_id}") if next_r_info.index != r_info.index + 1: return False app_src = vertex.app_vertex From 8767f5bf140f2e0f953a4ce95f2846a3b98ac738 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 10:54:24 +0100 Subject: [PATCH 07/16] Break things on purpose (to be undone) --- pacman/model/routing_info/routing_info.py | 21 +++-- .../routing_info_tests/test_routing_info.py | 90 ++++++++++--------- 2 files changed, 60 insertions(+), 51 deletions(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index aeab610e3..378572bdf 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -69,7 +69,10 @@ def get_routing_info_from_pre_vertex( The ID of the partition for which to get the routing information :rtype: VertexRoutingInfo or None """ - return self._info[vertex].get(partition_id) + # TODO: Replace (currently temporarily broken to make sure we don't + # call it) + raise NotImplementedError("Deprecated - shouldn't be used") + # return self._info[vertex].get(partition_id) def get_safe_routing_info_from_pre_vertex( self, vertex: AbstractVertex, @@ -106,12 +109,16 @@ def get_first_key_from_pre_vertex( :return: The routing key of the partition :rtype: int or None """ - if vertex not in self._info: - return None - info = self._info[vertex] - if partition_id not in info: - return None - return info[partition_id].key + # TODO: Replace (currently temporarily broken to make sure we don't + # call it) + raise NotImplementedError("Deprecated - shouldn't be used") + + # if vertex not in self._info: + # return None + # info = self._info[vertex] + # if partition_id not in info: + # return None + # return info[partition_id].key def get_safe_first_key_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> int: diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index a99789bd6..2480a4b16 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -28,50 +28,52 @@ class TestRoutingInfo(unittest.TestCase): def setUp(self): unittest_setup() - def test_routing_info_deprecated(self): - pre_vertex = SimpleMachineVertex(ConstantSDRAM(0)) - key = 12345 - info = MachineVertexRoutingInfo( - BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) - routing_info = RoutingInfo() - routing_info.add_routing_info(info) - - with self.assertRaises(PacmanAlreadyExistsException): - routing_info.add_routing_info(info) - - assert routing_info.get_routing_info_from_pre_vertex( - pre_vertex, "Test") == info - assert routing_info.get_routing_info_from_pre_vertex( - None, "Test") is None - assert routing_info.get_routing_info_from_pre_vertex( - pre_vertex, "None") is None - - assert routing_info.get_first_key_from_pre_vertex( - pre_vertex, "Test") == key - assert routing_info.get_first_key_from_pre_vertex( - None, "Test") is None - assert routing_info.get_first_key_from_pre_vertex( - pre_vertex, "None") is None - - assert next(iter(routing_info)) == info - - info2 = MachineVertexRoutingInfo( - BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) - - with self.assertRaises(PacmanAlreadyExistsException): - routing_info.add_routing_info(info2) - assert info != info2 - - info3 = MachineVertexRoutingInfo( - BaseKeyAndMask(key, FULL_MASK), "Test2", pre_vertex, 0) - routing_info.add_routing_info(info3) - assert info != info3 - assert routing_info.get_routing_info_from_pre_vertex( - pre_vertex, "Test2") !=\ - routing_info.get_routing_info_from_pre_vertex( - pre_vertex, "Test") - assert routing_info.get_routing_info_from_pre_vertex( - pre_vertex, "Test2").get_keys().tolist() == [key] + # TODO: Replace (currently temporarily broken to make sure we don't + # call it) + # def test_routing_info_deprecated(self): + # pre_vertex = SimpleMachineVertex(ConstantSDRAM(0)) + # key = 12345 + # info = MachineVertexRoutingInfo( + # BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) + # routing_info = RoutingInfo() + # routing_info.add_routing_info(info) + # + # with self.assertRaises(PacmanAlreadyExistsException): + # routing_info.add_routing_info(info) + # + # assert routing_info.get_routing_info_from_pre_vertex( + # pre_vertex, "Test") == info + # assert routing_info.get_routing_info_from_pre_vertex( + # None, "Test") is None + # assert routing_info.get_routing_info_from_pre_vertex( + # pre_vertex, "None") is None + # + # assert routing_info.get_first_key_from_pre_vertex( + # pre_vertex, "Test") == key + # assert routing_info.get_first_key_from_pre_vertex( + # None, "Test") is None + # assert routing_info.get_first_key_from_pre_vertex( + # pre_vertex, "None") is None + # + # assert next(iter(routing_info)) == info + # + # info2 = MachineVertexRoutingInfo( + # BaseKeyAndMask(key, FULL_MASK), "Test", pre_vertex, 0) + # + # with self.assertRaises(PacmanAlreadyExistsException): + # routing_info.add_routing_info(info2) + # assert info != info2 + # + # info3 = MachineVertexRoutingInfo( + # BaseKeyAndMask(key, FULL_MASK), "Test2", pre_vertex, 0) + # routing_info.add_routing_info(info3) + # assert info != info3 + # assert routing_info.get_routing_info_from_pre_vertex( + # pre_vertex, "Test2") !=\ + # routing_info.get_routing_info_from_pre_vertex( + # pre_vertex, "Test") + # assert routing_info.get_routing_info_from_pre_vertex( + # pre_vertex, "Test2").get_keys().tolist() == [key] def test_routing_info(self): pre_vertex = SimpleMachineVertex(ConstantSDRAM(0)) From 006709a67370454ce967a0673882ee15797c9adf Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 10:58:02 +0100 Subject: [PATCH 08/16] Remove unused import --- .../routing_table_generators/merged_routing_table_generator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pacman/operations/routing_table_generators/merged_routing_table_generator.py b/pacman/operations/routing_table_generators/merged_routing_table_generator.py index 863ce8a7f..571d7dc86 100644 --- a/pacman/operations/routing_table_generators/merged_routing_table_generator.py +++ b/pacman/operations/routing_table_generators/merged_routing_table_generator.py @@ -16,7 +16,6 @@ from spinn_utilities.progress_bar import ProgressBar from spinn_machine import MulticastRoutingEntry, RoutingEntry from pacman.data import PacmanDataView -from pacman.exceptions import PacmanRoutingException from pacman.model.routing_tables import ( UnCompressedMulticastRoutingTable, MulticastRoutingTables) from pacman.model.graphs.application import ApplicationVertex From 03400449bbd936f5cc3396032f488a76e71a5023 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 11:07:47 +0100 Subject: [PATCH 09/16] Fix this call too --- .../test_zoned_routing_allocator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py b/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py index 5ecf435f8..9d56aac7f 100644 --- a/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py +++ b/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py @@ -244,7 +244,7 @@ def check_keys_for_application_partition_pairs(routing_info, app_mask): mapped_key = None for m_vertex in part.pre_vertex.splitter.get_out_going_vertices( part.identifier): - key = routing_info.get_first_key_from_pre_vertex( + key = routing_info.get_safe_first_key_from_pre_vertex( m_vertex, part.identifier) if check_fixed(m_vertex, part.identifier, key): continue From 20160d3f64e4e04fde38d511f28083b4f739b5ca Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 11:14:21 +0100 Subject: [PATCH 10/16] Fix test --- .../routing_table_generator_tests/test_merged.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/operations_tests/routing_table_generator_tests/test_merged.py b/unittests/operations_tests/routing_table_generator_tests/test_merged.py index 2794912fa..e9b8ac18c 100644 --- a/unittests/operations_tests/routing_table_generator_tests/test_merged.py +++ b/unittests/operations_tests/routing_table_generator_tests/test_merged.py @@ -141,7 +141,7 @@ def test_bad_infos(self): try: merged_routing_table_generator() raise PacmanRoutingException("Should not get here") - except PacmanRoutingException as ex: + except KeyError as ex: self.assertIn("Missing Routing information", str(ex)) def test_iterator_with_next(self): From eb1d0adab8f3fc903b65c05ac830b2fe4cf37aba Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 11:15:13 +0100 Subject: [PATCH 11/16] Fixed --- .../routing_table_generator_tests/test_merged.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/operations_tests/routing_table_generator_tests/test_merged.py b/unittests/operations_tests/routing_table_generator_tests/test_merged.py index e9b8ac18c..bcbbc72e9 100644 --- a/unittests/operations_tests/routing_table_generator_tests/test_merged.py +++ b/unittests/operations_tests/routing_table_generator_tests/test_merged.py @@ -142,7 +142,7 @@ def test_bad_infos(self): merged_routing_table_generator() raise PacmanRoutingException("Should not get here") except KeyError as ex: - self.assertIn("Missing Routing information", str(ex)) + self.assertIn("foo", str(ex)) def test_iterator_with_next(self): empty = _IteratorWithNext([]) From 3b69f24585a6cc7725c743ffd04a00f6c9cddbf5 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 13:54:25 +0100 Subject: [PATCH 12/16] Add an extra utility method --- pacman/model/routing_info/routing_info.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 378572bdf..281e70245 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -57,6 +57,7 @@ def add_routing_info(self, info: VertexRoutingInfo): "Use a combination of " "get_safe_routing_info_from_pre_vertex, " "get_partitions_outgoing_from_vertex, " + "has_routing_info_from_pre_vertex, " "or get_single_routing_info_from_pre_vertex") def get_routing_info_from_pre_vertex( self, vertex: AbstractVertex, @@ -97,6 +98,7 @@ def get_safe_routing_info_from_pre_vertex( "Use a combination of " "get_safe_first_key_from_pre_vertex, " "get_partitions_outgoing_from_vertex, " + "has_routing_info_from_pre_vertex, " "or get_single_first_key_from_pre_vertex") def get_first_key_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> Optional[int]: @@ -145,6 +147,21 @@ def get_partitions_outgoing_from_vertex( """ return self._info[vertex].keys() + def has_routing_info_from_pre_vertex( + self, vertex: AbstractVertex, partition_id: str) -> bool: + """ + Check if there is routing information for a given vertex. + + :param AbstractVertex vertex: The vertex to search for + :param str partition_id: + The ID of the partition for which to get the routing information + :rtype: bool + """ + if vertex not in self._info: + return False + info = self._info[vertex] + return partition_id in info + def get_single_routing_info_from_pre_vertex( self, vertex: AbstractVertex) -> Optional[VertexRoutingInfo]: """ From 54b725054dfb9c55db8066ebe6937c5971e68523 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 13:57:06 +0100 Subject: [PATCH 13/16] Include new method in testing --- .../model_tests/routing_info_tests/test_routing_info.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 2480a4b16..7257c5feb 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -109,6 +109,13 @@ def test_routing_info(self): assert list(routing_info.get_partitions_outgoing_from_vertex( None)) == [] + assert routing_info.has_routing_info_from_pre_vertex( + pre_vertex, "Test") + assert not routing_info.has_routing_info_from_pre_vertex( + None, "Test") + assert not routing_info.has_routing_info_from_pre_vertex( + pre_vertex, "None") + assert next(iter(routing_info)) == info info2 = MachineVertexRoutingInfo( From f978bfcc1e43f5da4bae987700f69a4c14f6caad Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 14:03:53 +0100 Subject: [PATCH 14/16] More helpers! --- pacman/model/routing_info/routing_info.py | 20 ++++++++++++++++++- .../routing_info_tests/test_routing_info.py | 11 ++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 281e70245..130199b2e 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -13,7 +13,7 @@ # limitations under the License. from __future__ import annotations from collections import defaultdict -from typing import Dict, Iterator, Optional, Iterable, TYPE_CHECKING +from typing import Dict, Iterator, Optional, Iterable, Set, TYPE_CHECKING from deprecated import deprecated from pacman.exceptions import PacmanAlreadyExistsException if TYPE_CHECKING: @@ -162,6 +162,24 @@ def has_routing_info_from_pre_vertex( info = self._info[vertex] return partition_id in info + def test_pre_vertex_partition_ids( + self, vertex: AbstractVertex, + allowed_partition_ids: Set[str]): + """ + Check that the partition ids for a vertex are in the allowed set. + + :param AbstractVertex vertex: The vertex to search for + :param set[str] allowed_partition_ids: The allowed partition ids + :raise KeyError: If the vertex has an unknown partition ID + """ + if vertex not in self._info: + return + info = self._info[vertex] + for partition_id in info: + if partition_id not in allowed_partition_ids: + raise KeyError( + f"Vertex {vertex} has unknown partition ID {partition_id}") + def get_single_routing_info_from_pre_vertex( self, vertex: AbstractVertex) -> Optional[VertexRoutingInfo]: """ diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 7257c5feb..00b61e20b 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -109,6 +109,17 @@ def test_routing_info(self): assert list(routing_info.get_partitions_outgoing_from_vertex( None)) == [] + # This should work as can be either partition + routing_info.test_pre_vertex_partition_ids( + pre_vertex, {"Test", "Test2"}) + + # Works because None has no partitions! + routing_info.test_pre_vertex_partition_ids(None, {"Test"}) + + # This should not work + with self.assertRaises(KeyError): + routing_info.test_pre_vertex_partition_ids(pre_vertex, {"Test2"}) + assert routing_info.has_routing_info_from_pre_vertex( pre_vertex, "Test") assert not routing_info.has_routing_info_from_pre_vertex( From e45347368954a0bbb21b0014f185235670dfb960 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Thu, 10 Oct 2024 15:22:11 +0100 Subject: [PATCH 15/16] More tests --- .../model_tests/routing_info_tests/test_routing_info.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 00b61e20b..5da3dc1ca 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -146,6 +146,11 @@ def test_routing_info(self): pre_vertex, "Test") assert routing_info.get_safe_routing_info_from_pre_vertex( pre_vertex, "Test2").get_keys().tolist() == [key] + with self.assertRaises(KeyError): + routing_info.get_single_routing_info_from_pre_vertex( + pre_vertex) + with self.assertRaises(KeyError): + routing_info.get_single_first_key_from_pre_vertex(pre_vertex) def test_base_key_and_mask(self): with self.assertRaises(PacmanConfigurationException): From 210d8e9aafe437ed486389975d9df0ac95d6cfc4 Mon Sep 17 00:00:00 2001 From: Andrew Rowley Date: Mon, 14 Oct 2024 08:44:24 +0100 Subject: [PATCH 16/16] Rename functions --- pacman/model/routing_info/routing_info.py | 53 ++++++++----------- .../valid_routes_checker.py | 2 +- .../basic_routing_table_generator.py | 2 +- .../merged_routing_table_generator.py | 8 +-- .../routing_info_tests/test_routing_info.py | 38 ++++++------- .../test_zoned_routing_allocator.py | 2 +- 6 files changed, 49 insertions(+), 56 deletions(-) diff --git a/pacman/model/routing_info/routing_info.py b/pacman/model/routing_info/routing_info.py index 130199b2e..604cb67e5 100644 --- a/pacman/model/routing_info/routing_info.py +++ b/pacman/model/routing_info/routing_info.py @@ -55,10 +55,10 @@ def add_routing_info(self, info: VertexRoutingInfo): "outgoing edge, or if the outgoing edge is in another " "partition and the name is wrong. " "Use a combination of " - "get_safe_routing_info_from_pre_vertex, " - "get_partitions_outgoing_from_vertex, " - "has_routing_info_from_pre_vertex, " - "or get_single_routing_info_from_pre_vertex") + "get_info_from, " + "get_partitions_from, " + "has_info_from, " + "or get_single_info_from") def get_routing_info_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> Optional[VertexRoutingInfo]: @@ -70,12 +70,9 @@ def get_routing_info_from_pre_vertex( The ID of the partition for which to get the routing information :rtype: VertexRoutingInfo or None """ - # TODO: Replace (currently temporarily broken to make sure we don't - # call it) - raise NotImplementedError("Deprecated - shouldn't be used") - # return self._info[vertex].get(partition_id) + return self._info[vertex].get(partition_id) - def get_safe_routing_info_from_pre_vertex( + def get_info_from( self, vertex: AbstractVertex, partition_id: str) -> VertexRoutingInfo: """ @@ -96,10 +93,10 @@ def get_safe_routing_info_from_pre_vertex( "outgoing edge, or if the outgoing edge is in another " "partition and the name is wrong. " "Use a combination of " - "get_safe_first_key_from_pre_vertex, " - "get_partitions_outgoing_from_vertex, " - "has_routing_info_from_pre_vertex, " - "or get_single_first_key_from_pre_vertex") + "get_key_from, " + "get_partitions_from, " + "has_info_from, " + "or get_single_key_from") def get_first_key_from_pre_vertex( self, vertex: AbstractVertex, partition_id: str) -> Optional[int]: """ @@ -111,18 +108,14 @@ def get_first_key_from_pre_vertex( :return: The routing key of the partition :rtype: int or None """ - # TODO: Replace (currently temporarily broken to make sure we don't - # call it) - raise NotImplementedError("Deprecated - shouldn't be used") - - # if vertex not in self._info: - # return None - # info = self._info[vertex] - # if partition_id not in info: - # return None - # return info[partition_id].key + if vertex not in self._info: + return None + info = self._info[vertex] + if partition_id not in info: + return None + return info[partition_id].key - def get_safe_first_key_from_pre_vertex( + def get_key_from( self, vertex: AbstractVertex, partition_id: str) -> int: """ Get the first key for the partition starting at a vertex. @@ -138,7 +131,7 @@ def get_safe_first_key_from_pre_vertex( """ return self._info[vertex][partition_id].key - def get_partitions_outgoing_from_vertex( + def get_partitions_from( self, vertex: AbstractVertex) -> Iterable[str]: """ Get the outgoing partitions from a vertex. @@ -147,7 +140,7 @@ def get_partitions_outgoing_from_vertex( """ return self._info[vertex].keys() - def has_routing_info_from_pre_vertex( + def has_info_from( self, vertex: AbstractVertex, partition_id: str) -> bool: """ Check if there is routing information for a given vertex. @@ -162,7 +155,7 @@ def has_routing_info_from_pre_vertex( info = self._info[vertex] return partition_id in info - def test_pre_vertex_partition_ids( + def check_info_from( self, vertex: AbstractVertex, allowed_partition_ids: Set[str]): """ @@ -180,7 +173,7 @@ def test_pre_vertex_partition_ids( raise KeyError( f"Vertex {vertex} has unknown partition ID {partition_id}") - def get_single_routing_info_from_pre_vertex( + def get_single_info_from( self, vertex: AbstractVertex) -> Optional[VertexRoutingInfo]: """ Get routing information for a given vertex. Fails if the vertex has @@ -198,7 +191,7 @@ def get_single_routing_info_from_pre_vertex( f"Vertex {vertex} has more than one outgoing partition") return next(iter(info.values())) - def get_single_first_key_from_pre_vertex( + def get_single_key_from( self, vertex: AbstractVertex) -> Optional[int]: """ Get the first key for the partition starting at a vertex. Fails if @@ -208,7 +201,7 @@ def get_single_first_key_from_pre_vertex( :rtype: int or None :raise KeyError: If the vertex has more than one outgoing partition """ - info = self.get_single_routing_info_from_pre_vertex(vertex) + info = self.get_single_info_from(vertex) if info is None: return None return info.key diff --git a/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py b/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py index 0c4e9b591..9c59fc8fe 100644 --- a/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py +++ b/pacman/operations/multi_cast_router_check_functionality/valid_routes_checker.py @@ -118,7 +118,7 @@ def validate_routes(routing_tables: MulticastRoutingTables): if isinstance(m_vertex, AbstractVirtual): continue placement = PacmanDataView.get_placement_of_vertex(m_vertex) - r_info = routing_infos.get_safe_routing_info_from_pre_vertex( + r_info = routing_infos.get_info_from( m_vertex, partition.identifier) # search for these destinations diff --git a/pacman/operations/routing_table_generators/basic_routing_table_generator.py b/pacman/operations/routing_table_generators/basic_routing_table_generator.py index 524794f81..cf135da1c 100644 --- a/pacman/operations/routing_table_generators/basic_routing_table_generator.py +++ b/pacman/operations/routing_table_generators/basic_routing_table_generator.py @@ -63,7 +63,7 @@ def __create_routing_table( sources_by_key_mask: Dict[BaseKeyAndMask, Tuple[AbstractVertex, str]] = dict() for source_vertex, partition_id in partitions_in_table: - r_info = routing_infos.get_safe_routing_info_from_pre_vertex( + r_info = routing_infos.get_info_from( source_vertex, partition_id) entry = partitions_in_table[source_vertex, partition_id] if r_info.key_and_mask in sources_by_key_mask: diff --git a/pacman/operations/routing_table_generators/merged_routing_table_generator.py b/pacman/operations/routing_table_generators/merged_routing_table_generator.py index 571d7dc86..4b2362b52 100644 --- a/pacman/operations/routing_table_generators/merged_routing_table_generator.py +++ b/pacman/operations/routing_table_generators/merged_routing_table_generator.py @@ -69,7 +69,7 @@ def __create_routing_table( iterator = _IteratorWithNext(partitions_in_table.items()) while iterator.has_next: (vertex, part_id), entry = iterator.pop() - r_info = routing_info.get_safe_routing_info_from_pre_vertex( + r_info = routing_info.get_info_from( vertex, part_id) if r_info.key_and_mask in sources_by_key_mask: @@ -99,7 +99,7 @@ def __create_routing_table( continue # This has to be AppVertexRoutingInfo! - app_r_info = routing_info.get_safe_routing_info_from_pre_vertex( + app_r_info = routing_info.get_info_from( vertex.app_vertex, part_id) assert isinstance(app_r_info, AppVertexRoutingInfo) @@ -109,7 +109,7 @@ def __create_routing_table( while __match(iterator, vertex, part_id, r_info, entry, routing_info, app_r_info): (vertex, part_id), entry = iterator.pop() - r_info = routing_info.get_safe_routing_info_from_pre_vertex( + r_info = routing_info.get_info_from( vertex, part_id) if r_info is not None: assert isinstance(r_info, MachineVertexRoutingInfo) @@ -136,7 +136,7 @@ def __match( return False if __mask_has_holes(r_info.mask): return False - next_r_info = routing_info.get_safe_routing_info_from_pre_vertex( + next_r_info = routing_info.get_info_from( next_vertex, next_part_id) if next_r_info.index != r_info.index + 1: return False diff --git a/unittests/model_tests/routing_info_tests/test_routing_info.py b/unittests/model_tests/routing_info_tests/test_routing_info.py index 5da3dc1ca..c7f12edb8 100644 --- a/unittests/model_tests/routing_info_tests/test_routing_info.py +++ b/unittests/model_tests/routing_info_tests/test_routing_info.py @@ -86,45 +86,45 @@ def test_routing_info(self): with self.assertRaises(PacmanAlreadyExistsException): routing_info.add_routing_info(info) - assert routing_info.get_safe_routing_info_from_pre_vertex( + assert routing_info.get_info_from( pre_vertex, "Test") == info with self.assertRaises(KeyError): - routing_info.get_safe_routing_info_from_pre_vertex( + routing_info.get_info_from( None, "Test") with self.assertRaises(KeyError): - routing_info.get_safe_routing_info_from_pre_vertex( + routing_info.get_info_from( pre_vertex, "None") - assert routing_info.get_safe_first_key_from_pre_vertex( + assert routing_info.get_key_from( pre_vertex, "Test") == key with self.assertRaises(KeyError): - routing_info.get_safe_first_key_from_pre_vertex( + routing_info.get_key_from( None, "Test") with self.assertRaises(KeyError): - routing_info.get_safe_first_key_from_pre_vertex( + routing_info.get_key_from( pre_vertex, "None") - assert list(routing_info.get_partitions_outgoing_from_vertex( + assert list(routing_info.get_partitions_from( pre_vertex)) == ["Test"] - assert list(routing_info.get_partitions_outgoing_from_vertex( + assert list(routing_info.get_partitions_from( None)) == [] # This should work as can be either partition - routing_info.test_pre_vertex_partition_ids( + routing_info.check_info_from( pre_vertex, {"Test", "Test2"}) # Works because None has no partitions! - routing_info.test_pre_vertex_partition_ids(None, {"Test"}) + routing_info.check_info_from(None, {"Test"}) # This should not work with self.assertRaises(KeyError): - routing_info.test_pre_vertex_partition_ids(pre_vertex, {"Test2"}) + routing_info.check_info_from(pre_vertex, {"Test2"}) - assert routing_info.has_routing_info_from_pre_vertex( + assert routing_info.has_info_from( pre_vertex, "Test") - assert not routing_info.has_routing_info_from_pre_vertex( + assert not routing_info.has_info_from( None, "Test") - assert not routing_info.has_routing_info_from_pre_vertex( + assert not routing_info.has_info_from( pre_vertex, "None") assert next(iter(routing_info)) == info @@ -140,17 +140,17 @@ def test_routing_info(self): BaseKeyAndMask(key, FULL_MASK), "Test2", pre_vertex, 0) routing_info.add_routing_info(info3) assert info != info3 - assert routing_info.get_safe_routing_info_from_pre_vertex( + assert routing_info.get_info_from( pre_vertex, "Test2") !=\ - routing_info.get_safe_routing_info_from_pre_vertex( + routing_info.get_info_from( pre_vertex, "Test") - assert routing_info.get_safe_routing_info_from_pre_vertex( + assert routing_info.get_info_from( pre_vertex, "Test2").get_keys().tolist() == [key] with self.assertRaises(KeyError): - routing_info.get_single_routing_info_from_pre_vertex( + routing_info.get_single_info_from( pre_vertex) with self.assertRaises(KeyError): - routing_info.get_single_first_key_from_pre_vertex(pre_vertex) + routing_info.get_single_key_from(pre_vertex) def test_base_key_and_mask(self): with self.assertRaises(PacmanConfigurationException): diff --git a/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py b/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py index 9d56aac7f..4ed07886c 100644 --- a/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py +++ b/unittests/operations_tests/routing_info_algorithms_tests/test_zoned_routing_allocator.py @@ -244,7 +244,7 @@ def check_keys_for_application_partition_pairs(routing_info, app_mask): mapped_key = None for m_vertex in part.pre_vertex.splitter.get_out_going_vertices( part.identifier): - key = routing_info.get_safe_first_key_from_pre_vertex( + key = routing_info.get_key_from( m_vertex, part.identifier) if check_fixed(m_vertex, part.identifier, key): continue