From 407201a4ccd1c9365f3a18a93b7104e2fa1ef617 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 18 Mar 2024 09:06:15 +0000
Subject: [PATCH 01/33] _place_vertex

---
 .../placer_algorithms/application_placer.py   | 145 +++++++++---------
 1 file changed, 75 insertions(+), 70 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 5d5e5d77a..defe7d060 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -55,74 +55,13 @@ def place_application_graph(system_placements: Placements) -> Placements:
     # Go through the application graph by application vertex
     progress = ProgressBar(
         PacmanDataView.get_n_vertices(), "Placing Vertices")
-    for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
-        spaces.restore_chips()
-
-        # Try placements from the next chip, but try again if fails
-        placed = False
-        while not placed:
-            chips_attempted = list()
-            try:
-                same_chip_groups = app_vertex.splitter.get_same_chip_groups()
-                if not same_chip_groups:
-                    placed = True
-                    break
-
-                # Start a new space
-                try:
-                    next_chip_space, space = spaces.get_next_chip_and_space()
-                except PacmanPlaceException as e:
-                    raise _place_error(
-                        placements, system_placements, e,
-                        plan_n_timesteps) from e
-                logger.debug(f"Starting placement from {next_chip_space}")
-
-                placements_to_make: List = list()
-
-                # Go through the groups
-                last_chip_space: Optional[_ChipWithSpace] = None
-                for vertices, sdram in same_chip_groups:
-                    vertices_to_place = [
-                        vertex
-                        for vertex in vertices
-                        # No need to place virtual vertices
-                        if not isinstance(vertex, AbstractVirtual)
-                        and not placements.is_vertex_placed(vertex)]
-                    actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
-                    n_cores = len(vertices_to_place)
-
-                    # If this group has a fixed location, place it there
-                    if _do_fixed_location(vertices_to_place, actual_sdram,
-                                          placements, next_chip_space):
-                        continue
-
-                    # Try to find a chip with space; this might result in a
-                    # _SpaceExceededException
-                    while not next_chip_space.is_space(n_cores, actual_sdram):
-                        next_chip_space = spaces.get_next_chip_space(
-                            space, last_chip_space)
-                        last_chip_space = None
-
-                    # If this worked, store placements to be made
-                    last_chip_space = next_chip_space
-                    chips_attempted.append(next_chip_space.chip)
-                    _store_on_chip(
-                        placements_to_make, vertices_to_place, actual_sdram,
-                        next_chip_space)
-
-                # Now make the placements having confirmed all can be done
-                placements.add_placements(placements_to_make)
-                placed = True
-                logger.debug(f"Used {chips_attempted}")
-            except _SpaceExceededException:
-                # This might happen while exploring a space; this may not be
-                # fatal since the last space might have just been bound by
-                # existing placements, and there might be bigger spaces out
-                # there to use
-                _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
-                logger.debug(f"Failed, saving {chips_attempted}")
-                spaces.save_chips(chips_attempted)
-                chips_attempted.clear()
+    try:
+        for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
+            _place_vertex(app_vertex, spaces, plan_n_timesteps, placements)
+    except PacmanPlaceException as e:
+        raise _place_error(
+            placements, system_placements, e,
+            plan_n_timesteps) from e
 
     if get_config_bool("Reports", "draw_placements"):
         # pylint: disable=import-outside-toplevel
@@ -132,6 +71,73 @@ def place_application_graph(system_placements: Placements) -> Placements:
     return placements
 
 
+def _place_vertex(
+        app_vertex: ApplicationVertex, spaces: _Spaces,
+        plan_n_timesteps: Optional[int], placements: Placements):
+    spaces.restore_chips()
+
+    same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+    if not same_chip_groups:
+        # This vertex does not require placement or delegates
+        return
+
+    # Try placements from the next chip, but try again if fails
+    placed = False
+    while not placed:
+        chips_attempted = list()
+        try:
+            # Start a new space
+            next_chip_space, space = spaces.get_next_chip_and_space()
+            logger.debug(f"Starting placement from {next_chip_space}")
+
+            placements_to_make: List = list()
+
+            # Go through the groups
+            last_chip_space: Optional[_ChipWithSpace] = None
+            for vertices, sdram in same_chip_groups:
+                vertices_to_place = [
+                    vertex
+                    for vertex in vertices
+                    # No need to place virtual vertices
+                    if not isinstance(vertex, AbstractVirtual)
+                    and not placements.is_vertex_placed(vertex)]
+                actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
+                n_cores = len(vertices_to_place)
+
+                # If this group has a fixed location, place it there
+                if _do_fixed_location(vertices_to_place, actual_sdram,
+                                      placements, next_chip_space):
+                    continue
+
+                # Try to find a chip with space; this might result in a
+                # _SpaceExceededException
+                while not next_chip_space.is_space(n_cores, actual_sdram):
+                    next_chip_space = spaces.get_next_chip_space(
+                        space, last_chip_space)
+                    last_chip_space = None
+
+                # If this worked, store placements to be made
+                last_chip_space = next_chip_space
+                chips_attempted.append(next_chip_space.chip)
+                _store_on_chip(
+                    placements_to_make, vertices_to_place, actual_sdram,
+                    next_chip_space)
+
+            # Now make the placements having confirmed all can be done
+            placements.add_placements(placements_to_make)
+            placed = True
+            logger.debug(f"Used {chips_attempted}")
+        except _SpaceExceededException:
+            # This might happen while exploring a space; this may not be
+            # fatal since the last space might have just been bound by
+            # existing placements, and there might be bigger spaces out
+            # there to use
+            _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
+            logger.debug(f"Failed, saving {chips_attempted}")
+            spaces.save_chips(chips_attempted)
+            chips_attempted.clear()
+
+
 def _place_error(
         placements: Placements, system_placements: Placements,
         exception: Exception,
@@ -362,9 +368,8 @@ def __init__(
         self.__saved_chips: OrderedSet[Chip] = OrderedSet()
         self.__restored_chips: OrderedSet[Chip] = OrderedSet()
 
-    def __chip_order(self):
+    def __chip_order(self) -> Iterable[Chip]:
         """
-        :param Machine machine:
         :rtype: iterable(Chip)
         """
         s_x, s_y = get_config_str("Mapping", "placer_start_chip").split(",")

From c7751459b42b379e81bdbc6e71354b06229efa32 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 18 Mar 2024 15:37:02 +0000
Subject: [PATCH 02/33] use xy tuple (which now includes Chip)

---
 pacman/data/pacman_data_view.py               | 20 ++++++------
 pacman/model/placements/placements.py         | 31 ++++++++-----------
 .../fixed_route_router/fixed_route_router.py  |  2 +-
 .../placer_algorithms/application_placer.py   | 20 ++++++------
 .../placer_algorithms/draw_placements.py      | 16 +++++-----
 unittests/data/test_data.py                   |  6 ++--
 .../placement_tests/test_placements_model.py  |  4 +--
 7 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/pacman/data/pacman_data_view.py b/pacman/data/pacman_data_view.py
index c394d4323..a81ea7fef 100644
--- a/pacman/data/pacman_data_view.py
+++ b/pacman/data/pacman_data_view.py
@@ -15,11 +15,16 @@
 import logging
 from typing import (Iterable, List, Optional, Sequence, Type, TypeVar,
                     TYPE_CHECKING)
+
 from spinn_utilities.log import FormatAdapter
+from spinn_utilities.typing.coords import XY
+
 from spinn_machine.data import MachineDataView
+
 from pacman.exceptions import PacmanNotPlacedError
 from pacman.model.graphs.application import ApplicationGraph
 from pacman.model.resources import AbstractSDRAM, ConstantSDRAM
+
 if TYPE_CHECKING:
     from pacman.model.graphs import AbstractEdgePartition
     from pacman.model.graphs.application import (
@@ -360,29 +365,26 @@ def iterate_placements_by_vertex_type(
             iterate_placements_by_vertex_type(vertex_type)
 
     @classmethod
-    def iterate_placements_on_core(cls, x: int, y: int) -> Iterable[Placement]:
+    def iterate_placements_on_core(cls, xy: XY) -> Iterable[Placement]:
         """
         Iterate over placements with this x and y.
 
-        :param int x: x coordinate to find placements for.
-        :param int y: y coordinate to find placements for.
+        :parami tuple(int, int) xy: x and y coordinates to find placements for.
         :rtype: iterable(Placement)
         :raises ~spinn_utilities.exceptions.SpiNNUtilsException:
             If the placements are currently unavailable
         """
         if cls.__pacman_data._placements is None:
             raise cls._exception("placements")
-        return cls.__pacman_data._placements.iterate_placements_on_core(x, y)
+        return cls.__pacman_data._placements.iterate_placements_on_core(xy)
 
     @classmethod
     def iterate_placements_by_xy_and_type(
-            cls, x: int, y: int,
-            vertex_type: Type[VTX]) -> Iterable[Placement]:
+            cls, xy: XY, vertex_type: Type[VTX]) -> Iterable[Placement]:
         """
         Iterate over placements with this x, y and type.
 
-        :param int x: x coordinate to find placements for.
-        :param int y: y coordinate  to find placements for.
+        :param tuple(int, int) xy: x and y coordinates to find placements for.
         :param type vertex_type: Class of vertex to find
         :rtype: iterable(Placement)
         :raises ~spinn_utilities.exceptions.SpiNNUtilsException:
@@ -391,7 +393,7 @@ def iterate_placements_by_xy_and_type(
         if cls.__pacman_data._placements is None:
             raise cls._exception("placements")
         return cls.__pacman_data._placements.\
-            iterate_placements_by_xy_and_type(x, y, vertex_type)
+            iterate_placements_by_xy_and_type(xy, vertex_type)
 
     @classmethod
     def get_n_placements(cls) -> int:
diff --git a/pacman/model/placements/placements.py b/pacman/model/placements/placements.py
index 969267411..684740001 100644
--- a/pacman/model/placements/placements.py
+++ b/pacman/model/placements/placements.py
@@ -142,28 +142,25 @@ def is_processor_occupied(self, x: int, y: int, p: int) -> bool:
         """
         return (pr := self._placements.get((x, y))) is not None and p in pr
 
-    def iterate_placements_on_core(
-            self, x: int, y: int) -> Iterable[Placement]:
+    def iterate_placements_on_core(self, xy: XY) -> Iterable[Placement]:
         """
         Iterate over placements with this x and y.
 
-        :param int x: x coordinate to find placements for.
-        :param int y: y coordinate  to find placements for.
+        :param tuple(int, int) xy: x and y coordinates to find placements for.
         :rtype: iterable(Placement)
         """
-        return self._placements[x, y].values()
+        return self._placements[xy].values()
 
     def iterate_placements_by_xy_and_type(
-            self, x: int, y: int, vertex_type: type) -> Iterable[Placement]:
+            self, xy: XY, vertex_type: type) -> Iterable[Placement]:
         """
         Iterate over placements with this x, y and this vertex_type.
 
-        :param int x: x coordinate to find placements for.
-        :param int y: y coordinate  to find placements for.
+        :param tuple(int, int) xy: x and y coordinate to find placements for.
         :param class vertex_type: Class of vertex to find
         :rtype: iterable(Placement)
         """
-        for placement in self._placements[x, y].values():
+        for placement in self._placements[xy].values():
             if isinstance(placement.vertex, vertex_type):
                 yield placement
 
@@ -179,17 +176,16 @@ def iterate_placements_by_vertex_type(
             if isinstance(placement.vertex, vertex_type):
                 yield placement
 
-    def n_placements_on_chip(self, x: int, y: int) -> int:
+    def n_placements_on_chip(self, xy: XY) -> int:
         """
         The number of placements on the given chip.
 
-        :param int x: x coordinate of chip.
-        :param int y: y coordinate of chip.
+        :param tuple(int, int) xy: x and y coordinate of chip.
         :rtype: int
         """
-        if (x, y) not in self._placements:
+        if xy not in self._placements:
             return 0
-        return len(self._placements[x, y])
+        return len(self._placements[xy])
 
     @property
     def placements(self) -> Iterable[Placement]:
@@ -201,15 +197,14 @@ def placements(self) -> Iterable[Placement]:
         """
         return iter(self._machine_vertices.values())
 
-    def placements_on_chip(self, x: int, y: int) -> Iterable[Placement]:
+    def placements_on_chip(self, xy: XY) -> Iterable[Placement]:
         """
         Get the placements on a specific chip.
 
-        :param int x: The x-coordinate of the chip
-        :param int y: The y-coordinate of the chip
+        :param tuple(int , int) xy: The x and y coordinates of the chip
         :rtype: iterable(Placement)
         """
-        return self._placements[x, y].values()
+        return self._placements[xy].values()
 
     @property
     def chips_with_placements(self) -> Iterable[XY]:
diff --git a/pacman/operations/fixed_route_router/fixed_route_router.py b/pacman/operations/fixed_route_router/fixed_route_router.py
index c46ac9e74..41160dd7c 100644
--- a/pacman/operations/fixed_route_router/fixed_route_router.py
+++ b/pacman/operations/fixed_route_router/fixed_route_router.py
@@ -150,7 +150,7 @@ def __locate_destination(self, chip: Chip) -> int:
         :raises PacmanConfigurationException: if no placement processor found
         """
         for placement in PacmanDataView.iterate_placements_by_xy_and_type(
-                chip.x, chip.y, self._destination_class):
+                chip, self._destination_class):
             return placement.p
         raise PacmanConfigurationException(
             f"no destination vertex found on Ethernet chip {chip.x}:{chip.y}")
diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 5d5e5d77a..40f469d9d 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -173,13 +173,13 @@ def _place_error(
                 f"{PacmanDataView.get_n_vertices()} application vertices.\n")
         f.write(f"    Could not place {vertex_count} of {n_vertices} in the"
                 " last app vertex\n\n")
-        for x, y in placements.chips_with_placements:
+        for xy in placements.chips_with_placements:
             first = True
-            for placement in placements.placements_on_chip(x, y):
+            for placement in placements.placements_on_chip(xy):
                 if system_placements.is_vertex_placed(placement.vertex):
                     continue
                 if first:
-                    f.write(f"Chip ({x}, {y}):\n")
+                    f.write(f"Chip ({xy}):\n")
                     first = False
                 f.write(f"    Processor {placement.p}:"
                         f" Vertex {placement.vertex}\n")
@@ -204,12 +204,12 @@ def _place_error(
         f.write("\n")
         f.write("Unused chips:\n")
         machine = PacmanDataView.get_machine()
-        for x, y in machine.chip_coordinates:
-            n_placed = placements.n_placements_on_chip(x, y)
-            system_placed = system_placements.n_placements_on_chip(x, y)
+        for xy in machine.chip_coordinates:
+            n_placed = placements.n_placements_on_chip(xy)
+            system_placed = system_placements.n_placements_on_chip(xy)
             if n_placed - system_placed == 0:
-                n_procs = machine[x, y].n_placable_processors
-                f.write(f"    {x}, {y} ({n_procs - system_placed}"
+                n_procs = machine[xy].n_placable_processors
+                f.write(f"    {xy} ({n_procs - system_placed}"
                         " free cores)\n")
 
     if get_config_bool("Reports", "draw_placements_on_error"):
@@ -298,7 +298,7 @@ def _do_fixed_location(
     if chip is None:
         raise PacmanConfigurationException(
             f"Constrained to chip {x, y} but no such chip")
-    on_chip = placements.placements_on_chip(x, y)
+    on_chip = placements.placements_on_chip(chip)
     cores_used = {p.p for p in on_chip}
     cores = set(chip.placable_processors_ids) - cores_used
     next_cores = iter(cores)
@@ -385,7 +385,7 @@ def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
         :return cores, sdram
         :rtype: tuple(int, int)
         """
-        on_chip = self.__placements.placements_on_chip(chip.x, chip.y)
+        on_chip = self.__placements.placements_on_chip(chip)
         cores_used = {p.p for p in on_chip}
         sdram_used = sum(
             p.vertex.sdram_required.get_total_sdram(
diff --git a/pacman/operations/placer_algorithms/draw_placements.py b/pacman/operations/placer_algorithms/draw_placements.py
index f26a01ab5..68072ab72 100644
--- a/pacman/operations/placer_algorithms/draw_placements.py
+++ b/pacman/operations/placer_algorithms/draw_placements.py
@@ -66,18 +66,16 @@ def draw_placements(
     vertex_colours[None] = unused
     board_colours: Dict[XY, Colour] = dict()
     machine = PacmanDataView.get_machine()
-    for x, y in machine.chip_coordinates:
-        if (placements.n_placements_on_chip(x, y) ==
-                system_placements.n_placements_on_chip(x, y)):
-            board_colours[x, y] = unused
+    for xy in machine.chip_coordinates:
+        if (placements.n_placements_on_chip(xy) ==
+                system_placements.n_placements_on_chip(xy)):
+            board_colours[xy] = unused
             continue
-        for placement in placements.placements_on_chip(x, y):
+        for placement in placements.placements_on_chip(xy):
             if not system_placements.is_vertex_placed(placement.vertex):
-                board_colours[x, y] = \
+                board_colours[xy] = \
                     vertex_colours[placement.vertex.app_vertex]
                 break
-    include_boards = [
-        (chip.x, chip.y) for chip in machine.ethernet_connected_chips]
 
     # Compute dimensions
     w = math.ceil(machine.width / 12)
@@ -91,4 +89,4 @@ def draw_placements(
             report_file, image_width, image_height) as ctx:
         spinner_api.draw(
             ctx, image_width, image_height, machine.width, machine.height,
-            hex_boards, {}, board_colours, include_boards)
+            hex_boards, {}, board_colours, machine.ethernet_connected_chips)
diff --git a/unittests/data/test_data.py b/unittests/data/test_data.py
index 8ad9199b0..50e83f1a1 100644
--- a/unittests/data/test_data.py
+++ b/unittests/data/test_data.py
@@ -180,9 +180,9 @@ def test_placements_safety_code(self):
         with self.assertRaises(DataNotYetAvialable):
             writer.iterate_placements_by_vertex_type(None)
         with self.assertRaises(DataNotYetAvialable):
-            writer.iterate_placements_on_core(None, None)
+            writer.iterate_placements_on_core((None, None))
         with self.assertRaises(DataNotYetAvialable):
-            writer.iterate_placements_by_xy_and_type(None, None, None)
+            writer.iterate_placements_by_xy_and_type((None, None), None)
         with self.assertRaises(DataNotYetAvialable):
             PacmanDataView.get_n_placements()
         with self.assertRaises(DataNotYetAvialable):
@@ -201,7 +201,7 @@ def test_placements(self):
         info.add_placement(Placement(SimpleMachineVertex(None), 2, 2, 3))
         writer.set_placements(info)
         self.assertEqual(3, PacmanDataView.get_n_placements())
-        on12 = list(PacmanDataView.iterate_placements_on_core(1, 2))
+        on12 = list(PacmanDataView.iterate_placements_on_core((1, 2)))
         self.assertEqual(on12, [p1, p2])
         vertex = PacmanDataView.get_placement_on_processor(1, 2, 5).vertex
         self.assertEqual(v2, vertex)
diff --git a/unittests/model_tests/placement_tests/test_placements_model.py b/unittests/model_tests/placement_tests/test_placements_model.py
index b29b105cb..8f3138e90 100644
--- a/unittests/model_tests/placement_tests/test_placements_model.py
+++ b/unittests/model_tests/placement_tests/test_placements_model.py
@@ -126,8 +126,8 @@ def test_infos_code(self):
         self.assertTrue(pls.is_processor_occupied(0, 0, 1))
         self.assertFalse(pls.is_processor_occupied(0, 0, 3))
         self.assertFalse(pls.is_processor_occupied(0, 1, 1))
-        self.assertEqual(2, pls.n_placements_on_chip(0, 0))
-        self.assertEqual(0, pls.n_placements_on_chip(0, 2))
+        self.assertEqual(2, pls.n_placements_on_chip((0, 0)))
+        self.assertEqual(0, pls.n_placements_on_chip((0, 2)))
         self.assertListEqual([(0, 0)], list(pls.chips_with_placements))
         self.assertEqual("(0, 0)", repr(pls))
         self.assertEqual(2, len(pls))

From a10ed57f748457760ff1f934e0186327f9da87f8 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 18 Mar 2024 16:12:57 +0000
Subject: [PATCH 03/33] pylint

---
 pacman/data/pacman_data_view.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pacman/data/pacman_data_view.py b/pacman/data/pacman_data_view.py
index a81ea7fef..7b51c7101 100644
--- a/pacman/data/pacman_data_view.py
+++ b/pacman/data/pacman_data_view.py
@@ -369,7 +369,7 @@ def iterate_placements_on_core(cls, xy: XY) -> Iterable[Placement]:
         """
         Iterate over placements with this x and y.
 
-        :parami tuple(int, int) xy: x and y coordinates to find placements for.
+        :param tuple(int, int) xy: x and y coordinates to find placements for.
         :rtype: iterable(Placement)
         :raises ~spinn_utilities.exceptions.SpiNNUtilsException:
             If the placements are currently unavailable

From 3acba0f7ee10757b7f14c0da20a415a36ac472bf Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 18 Mar 2024 17:32:22 +0000
Subject: [PATCH 04/33] comments

---
 .../placer_algorithms/application_placer.py   | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 7afc6ee9d..b09bfbf0f 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -179,13 +179,13 @@ def _place_error(
                 f"{PacmanDataView.get_n_vertices()} application vertices.\n")
         f.write(f"    Could not place {vertex_count} of {n_vertices} in the"
                 " last app vertex\n\n")
-        for xy in placements.chips_with_placements:
+        for x, y in placements.chips_with_placements:
             first = True
-            for placement in placements.placements_on_chip(xy):
+            for placement in placements.placements_on_chip(x, y):
                 if system_placements.is_vertex_placed(placement.vertex):
                     continue
                 if first:
-                    f.write(f"Chip ({xy}):\n")
+                    f.write(f"Chip ({x}, {y}):\n")
                     first = False
                 f.write(f"    Processor {placement.p}:"
                         f" Vertex {placement.vertex}\n")
@@ -210,12 +210,12 @@ def _place_error(
         f.write("\n")
         f.write("Unused chips:\n")
         machine = PacmanDataView.get_machine()
-        for xy in machine.chip_coordinates:
-            n_placed = placements.n_placements_on_chip(xy)
-            system_placed = system_placements.n_placements_on_chip(xy)
+        for x, y in machine.chip_coordinates:
+            n_placed = placements.n_placements_on_chip(x, y)
+            system_placed = system_placements.n_placements_on_chip(x, y)
             if n_placed - system_placed == 0:
-                n_procs = machine[xy].n_placable_processors
-                f.write(f"    {xy} ({n_procs - system_placed}"
+                n_procs = machine[x, y].n_placable_processors
+                f.write(f"    {x}, {y} ({n_procs - system_placed}"
                         " free cores)\n")
 
     if get_config_bool("Reports", "draw_placements_on_error"):
@@ -304,7 +304,7 @@ def _do_fixed_location(
     if chip is None:
         raise PacmanConfigurationException(
             f"Constrained to chip {x, y} but no such chip")
-    on_chip = placements.placements_on_chip(chip)
+    on_chip = placements.placements_on_chip(x, y)
     cores_used = {p.p for p in on_chip}
     cores = set(chip.placable_processors_ids) - cores_used
     next_cores = iter(cores)
@@ -386,6 +386,8 @@ def __chip_order(self) -> Iterable[Chip]:
 
     def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
         """
+        Gets the core ids and the plan sdram of the placements for this Chip
+
         :param Chip chip:
         :return cores, sdram
         :rtype: tuple(int, int)
@@ -421,6 +423,10 @@ def get_next_chip_and_space(self) -> Tuple[_ChipWithSpace, _Space]:
 
     def __get_next_chip(self) -> Chip:
         """
+        Gets the next Chip from either restored or if none the Machine
+
+        Ignores any Chip in the used set
+
         :rtype: Chip
         :raises: StopIteration
         """

From 0d367a363241ed28a2ea6f4b91e0dd09d79d5885 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 05:54:39 +0000
Subject: [PATCH 05/33] revert to converting machine.ethernet_connected_chips
 to xy list

---
 pacman/operations/placer_algorithms/draw_placements.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/draw_placements.py b/pacman/operations/placer_algorithms/draw_placements.py
index 68072ab72..f27681a06 100644
--- a/pacman/operations/placer_algorithms/draw_placements.py
+++ b/pacman/operations/placer_algorithms/draw_placements.py
@@ -76,6 +76,8 @@ def draw_placements(
                 board_colours[xy] = \
                     vertex_colours[placement.vertex.app_vertex]
                 break
+    include_boards = [
+        (chip.x, chip.y) for chip in machine.ethernet_connected_chips]
 
     # Compute dimensions
     w = math.ceil(machine.width / 12)
@@ -89,4 +91,4 @@ def draw_placements(
             report_file, image_width, image_height) as ctx:
         spinner_api.draw(
             ctx, image_width, image_height, machine.width, machine.height,
-            hex_boards, {}, board_colours, machine.ethernet_connected_chips)
+            hex_boards, {}, board_colours, include_boards)

From 67ce0c4b406c474e9dbbc28e7db0d7d707155b41 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 08:20:28 +0000
Subject: [PATCH 06/33] place fixed location fist

---
 .../graphs/application/application_vertex.py  | 16 +++++
 .../placer_algorithms/application_placer.py   | 60 ++++++++++++-------
 .../test_application_placer.py                |  6 +-
 3 files changed, 56 insertions(+), 26 deletions(-)

diff --git a/pacman/model/graphs/application/application_vertex.py b/pacman/model/graphs/application/application_vertex.py
index e74277e18..ba33ea5cc 100644
--- a/pacman/model/graphs/application/application_vertex.py
+++ b/pacman/model/graphs/application/application_vertex.py
@@ -440,3 +440,19 @@ def get_raster_ordered_indices(self, indices):
             cum_size *= atoms_shape[n]
 
         return global_index.astype(numpy.uint32)
+
+    def has_fixed_location(self):
+        """
+        Check if this vertex or any machine vertex has a fixed location.
+
+        :rtype: bool
+        :returns: True if the Application Vertex or any one of its
+        Machine Vertices has a fixed location
+        False if None of the Vertices has a none None fixed location
+        """
+        if self.get_fixed_location() is not None:
+            return True
+        for vertex in self._machine_vertices:
+            if vertex.get_fixed_location() is not None:
+                return True
+        return False
diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index b09bfbf0f..97cc3dbc6 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -50,13 +50,18 @@ def place_application_graph(system_placements: Placements) -> Placements:
     placements = Placements(system_placements)
 
     plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
-    spaces = _Spaces(placements, plan_n_timesteps)
-
     # Go through the application graph by application vertex
     progress = ProgressBar(
-        PacmanDataView.get_n_vertices(), "Placing Vertices")
+        PacmanDataView.get_n_vertices() * 2, "Placing Vertices")
     try:
+        for app_vertex in progress.over(
+                PacmanDataView.iterate_vertices(), finish_at_end=False):
+            if app_vertex.has_fixed_location():
+                _place_fixed_vertex(
+                    app_vertex, plan_n_timesteps, placements)
+        spaces = _Spaces(placements, plan_n_timesteps)
         for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
+            # as this checks if placed already not need to check if fixed
             _place_vertex(app_vertex, spaces, plan_n_timesteps, placements)
     except PacmanPlaceException as e:
         raise _place_error(
@@ -104,11 +109,6 @@ def _place_vertex(
                 actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
                 n_cores = len(vertices_to_place)
 
-                # If this group has a fixed location, place it there
-                if _do_fixed_location(vertices_to_place, actual_sdram,
-                                      placements, next_chip_space):
-                    continue
-
                 # Try to find a chip with space; this might result in a
                 # _SpaceExceededException
                 while not next_chip_space.is_space(n_cores, actual_sdram):
@@ -280,9 +280,27 @@ class _SpaceExceededException(Exception):
     pass
 
 
+def _place_fixed_vertex(
+        app_vertex: ApplicationVertex,
+        plan_n_timesteps: Optional[int], placements: Placements):
+    same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+    if not same_chip_groups:
+        raise NotImplementedError("Unexpected mix of Fixed and no groups")
+
+    for vertices, sdram in same_chip_groups:
+        vertices_to_place = [vertex
+            for vertex in vertices
+            # No need to place virtual vertices
+            if not isinstance(vertex, AbstractVirtual)
+            and not placements.is_vertex_placed(vertex)]
+
+        actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
+
+        _do_fixed_location(vertices_to_place, actual_sdram, placements)
+
+
 def _do_fixed_location(
-        vertices: list[MachineVertex], sdram: int, placements: Placements,
-        next_chip_space: _ChipWithSpace) -> bool:
+        vertices: list[MachineVertex], sdram: int, placements: Placements):
     """
     :param list(MachineVertex) vertices:
     :param int sdram:
@@ -297,27 +315,31 @@ def _do_fixed_location(
             x, y = loc.x, loc.y
             break
     else:
-        return False
+        raise NotImplementedError(
+            "Mixing fixed location and not fixed location groups "
+            "within one vertex")
 
-    machine = PacmanDataView.get_machine()
-    chip = machine.get_chip_at(x, y)
+    chip = PacmanDataView.get_chip_at(x, y)
     if chip is None:
         raise PacmanConfigurationException(
             f"Constrained to chip {x, y} but no such chip")
-    on_chip = placements.placements_on_chip(x, y)
+    on_chip = placements.placements_on_chip(chip)
     cores_used = {p.p for p in on_chip}
     cores = set(chip.placable_processors_ids) - cores_used
     next_cores = iter(cores)
+    # first do the ones with a fixed p
     for vertex in vertices:
-        next_core = None
         fixed = vertex.get_fixed_location()
         if fixed and fixed.p is not None:
             if fixed.p not in next_cores:
                 raise PacmanConfigurationException(
                     f"Core {fixed.p} on {x}, {y} not available to "
                     f"place {vertex} on")
-            next_core = fixed.p
-        else:
+            placements.add_placement(Placement(vertex, x, y, fixed.p))
+    # Then do the ones without a fixed p
+    for vertex in vertices:
+        fixed = vertex.get_fixed_location()
+        if not fixed or fixed.p is None:
             try:
                 next_core = next(next_cores)
             except StopIteration:
@@ -325,10 +347,6 @@ def _do_fixed_location(
                 raise PacmanConfigurationException(
                     f"No more cores available on {x}, {y}: {on_chip}")
         placements.add_placement(Placement(vertex, x, y, next_core))
-        if next_chip_space.x == x and next_chip_space.y == y:
-            next_chip_space.cores.remove(next_core)
-            next_chip_space.use_sdram(sdram)
-    return True
 
 
 def _store_on_chip(
diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index e9a9f7125..266caa81e 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -124,11 +124,7 @@ def test_application_placer_late_fixed():
     fixed.splitter.create_machine_vertices(ChipCounter())
 
     writer.set_machine(virtual_machine(24, 12))
-    try:
-        place_application_graph(Placements())
-    except PacmanConfigurationException:
-        raise unittest.SkipTest(
-            "https://github.com/SpiNNakerManchester/PACMAN/issues/444")
+    place_application_graph(Placements())
 
 
 def test_sdram_bigger_than_chip():

From 17166ad2c9bf4e35dfb77832974e0a716213e806 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 08:25:58 +0000
Subject: [PATCH 07/33] flake8

---
 pacman/operations/placer_algorithms/application_placer.py   | 6 +++---
 .../placer_algorithms_tests/test_application_placer.py      | 5 +----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 97cc3dbc6..c8cd6ac96 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -288,11 +288,11 @@ def _place_fixed_vertex(
         raise NotImplementedError("Unexpected mix of Fixed and no groups")
 
     for vertices, sdram in same_chip_groups:
-        vertices_to_place = [vertex
-            for vertex in vertices
+        vertices_to_place = [
+            vertex for vertex in vertices
             # No need to place virtual vertices
             if not isinstance(vertex, AbstractVirtual)
-            and not placements.is_vertex_placed(vertex)]
+               and not placements.is_vertex_placed(vertex)]
 
         actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
 
diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index 266caa81e..04a4a5a66 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -11,13 +11,10 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import unittest
-
 from spinn_utilities.config_holder import set_config
 from spinn_machine.virtual_machine import virtual_machine
 from pacman.data.pacman_data_writer import PacmanDataWriter
-from pacman.exceptions import (
-    PacmanConfigurationException, PacmanTooBigToPlace)
+from pacman.exceptions import PacmanTooBigToPlace
 from pacman.model.partitioner_splitters import (
     SplitterFixedLegacy, AbstractSplitterCommon)
 from pacman.operations.placer_algorithms.application_placer import (

From 2d5a0b8bf16a577bca3c8e3b0270abe0888a9c1e Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 08:31:49 +0000
Subject: [PATCH 08/33] allignment fix attempt 2

---
 pacman/operations/placer_algorithms/application_placer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index c8cd6ac96..88469c71a 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -292,7 +292,7 @@ def _place_fixed_vertex(
             vertex for vertex in vertices
             # No need to place virtual vertices
             if not isinstance(vertex, AbstractVirtual)
-               and not placements.is_vertex_placed(vertex)]
+            and not placements.is_vertex_placed(vertex)]
 
         actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
 

From 177e9e1d24bdb76ef45335a2e2631a2c17fbfb7c Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 08:39:42 +0000
Subject: [PATCH 09/33] remove unused param

---
 .../placer_algorithms/application_placer.py     | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 88469c71a..6c8c60f7a 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -49,7 +49,6 @@ def place_application_graph(system_placements: Placements) -> Placements:
     # Track the placements and space
     placements = Placements(system_placements)
 
-    plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
     # Go through the application graph by application vertex
     progress = ProgressBar(
         PacmanDataView.get_n_vertices() * 2, "Placing Vertices")
@@ -57,8 +56,9 @@ def place_application_graph(system_placements: Placements) -> Placements:
         for app_vertex in progress.over(
                 PacmanDataView.iterate_vertices(), finish_at_end=False):
             if app_vertex.has_fixed_location():
-                _place_fixed_vertex(
-                    app_vertex, plan_n_timesteps, placements)
+                _place_fixed_vertex(app_vertex, placements)
+
+        plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
         spaces = _Spaces(placements, plan_n_timesteps)
         for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
             # as this checks if placed already not need to check if fixed
@@ -281,8 +281,7 @@ class _SpaceExceededException(Exception):
 
 
 def _place_fixed_vertex(
-        app_vertex: ApplicationVertex,
-        plan_n_timesteps: Optional[int], placements: Placements):
+        app_vertex: ApplicationVertex, placements: Placements):
     same_chip_groups = app_vertex.splitter.get_same_chip_groups()
     if not same_chip_groups:
         raise NotImplementedError("Unexpected mix of Fixed and no groups")
@@ -294,18 +293,14 @@ def _place_fixed_vertex(
             if not isinstance(vertex, AbstractVirtual)
             and not placements.is_vertex_placed(vertex)]
 
-        actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
-
-        _do_fixed_location(vertices_to_place, actual_sdram, placements)
+        _do_fixed_location(vertices_to_place, placements)
 
 
 def _do_fixed_location(
-        vertices: list[MachineVertex], sdram: int, placements: Placements):
+        vertices: list[MachineVertex], placements: Placements):
     """
     :param list(MachineVertex) vertices:
-    :param int sdram:
     :param Placements placements:
-    :param _ChipWithSpace next_chip_space:
     :rtype: bool
     :raise PacmanConfigurationException:
     """

From 1c9509791aea15068b8035985f7146e13d200a1d Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 08:43:09 +0000
Subject: [PATCH 10/33] remove unused variable

---
 pacman/operations/placer_algorithms/application_placer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 6c8c60f7a..5f76529df 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -286,7 +286,7 @@ def _place_fixed_vertex(
     if not same_chip_groups:
         raise NotImplementedError("Unexpected mix of Fixed and no groups")
 
-    for vertices, sdram in same_chip_groups:
+    for vertices, _ in same_chip_groups:
         vertices_to_place = [
             vertex for vertex in vertices
             # No need to place virtual vertices

From d219c2bf6f5a6c3ee707cfee20a89694183b2ac4 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 09:32:50 +0000
Subject: [PATCH 11/33] use default chip order

---
 .../placer_algorithms/application_placer.py   | 34 +++++--------------
 pacman/pacman.cfg                             |  1 -
 2 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 5f76529df..180ddd158 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -179,13 +179,13 @@ def _place_error(
                 f"{PacmanDataView.get_n_vertices()} application vertices.\n")
         f.write(f"    Could not place {vertex_count} of {n_vertices} in the"
                 " last app vertex\n\n")
-        for x, y in placements.chips_with_placements:
+        for xy in placements.chips_with_placements:
             first = True
-            for placement in placements.placements_on_chip(x, y):
+            for placement in placements.placements_on_chip(xy):
                 if system_placements.is_vertex_placed(placement.vertex):
                     continue
                 if first:
-                    f.write(f"Chip ({x}, {y}):\n")
+                    f.write(f"Chip ({xy}):\n")
                     first = False
                 f.write(f"    Processor {placement.p}:"
                         f" Vertex {placement.vertex}\n")
@@ -210,12 +210,12 @@ def _place_error(
         f.write("\n")
         f.write("Unused chips:\n")
         machine = PacmanDataView.get_machine()
-        for x, y in machine.chip_coordinates:
-            n_placed = placements.n_placements_on_chip(x, y)
-            system_placed = system_placements.n_placements_on_chip(x, y)
+        for xy in machine.chip_coordinates:
+            n_placed = placements.n_placements_on_chip(xy)
+            system_placed = system_placements.n_placements_on_chip(xy)
             if n_placed - system_placed == 0:
-                n_procs = machine[x, y].n_placable_processors
-                f.write(f"    {x}, {y} ({n_procs - system_placed}"
+                n_procs = machine[xy].n_placable_processors
+                f.write(f"    {xy} ({n_procs - system_placed}"
                         " free cores)\n")
 
     if get_config_bool("Reports", "draw_placements_on_error"):
@@ -374,29 +374,13 @@ def __init__(
         self.__machine = PacmanDataView.get_machine()
         self.__placements = placements
         self.__plan_n_timesteps = plan_n_timesteps
-        self.__chips = iter(self.__chip_order())
+        self.__chips = self.__machine.chips
         self.__next_chip = next(self.__chips)
         self.__used_chips: Set[Chip] = set()
         self.__last_chip_space: Optional[_ChipWithSpace] = None
         self.__saved_chips: OrderedSet[Chip] = OrderedSet()
         self.__restored_chips: OrderedSet[Chip] = OrderedSet()
 
-    def __chip_order(self) -> Iterable[Chip]:
-        """
-        :rtype: iterable(Chip)
-        """
-        s_x, s_y = get_config_str("Mapping", "placer_start_chip").split(",")
-        s_x = int(s_x)
-        s_y = int(s_y)
-
-        for x in range(self.__machine.width):
-            for y in range(self.__machine.height):
-                c_x = (x + s_x) % self.__machine.width
-                c_y = (y + s_y) % self.__machine.height
-                chip = self.__machine.get_chip_at(c_x, c_y)
-                if chip:
-                    yield chip
-
     def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
         """
         Gets the core ids and the plan sdram of the placements for this Chip
diff --git a/pacman/pacman.cfg b/pacman/pacman.cfg
index b3a93217f..1c031d96e 100644
--- a/pacman/pacman.cfg
+++ b/pacman/pacman.cfg
@@ -16,4 +16,3 @@ draw_placements_on_error = False
 
 [Mapping]
 router_table_compress_as_far_as_possible = False
-placer_start_chip = 0,0

From bc955021fc1051f384b61b8ec7b75412f89e7f12 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 09:56:09 +0000
Subject: [PATCH 12/33] seperate two functions

---
 .../placer_algorithms/application_placer.py          | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 180ddd158..d7a883c77 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -92,7 +92,8 @@ def _place_vertex(
         chips_attempted = list()
         try:
             # Start a new space
-            next_chip_space, space = spaces.get_next_chip_and_space()
+            next_chip_space, space = spaces.get_next_chip()
+            space = _Space(next_chip_space)
             logger.debug(f"Starting placement from {next_chip_space}")
 
             placements_to_make: List = list()
@@ -396,9 +397,9 @@ def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
                 self.__plan_n_timesteps) for p in on_chip)
         return cores_used, sdram_used
 
-    def get_next_chip_and_space(self) -> Tuple[_ChipWithSpace, _Space]:
+    def get_next_chip(self) -> _ChipWithSpace:
         """
-        :rtype: (_ChipWithSpace, _Space)
+        :rtype: _ChipWithSpace
         """
         try:
             if self.__last_chip_space is None:
@@ -408,10 +409,7 @@ def get_next_chip_and_space(self) -> Tuple[_ChipWithSpace, _Space]:
                     chip, cores_used, sdram_used)
                 self.__used_chips.add(chip)
 
-            # Start a new space by finding all the chips that can be reached
-            # from the start chip but have not been used
-            return (self.__last_chip_space,
-                    _Space(self.__last_chip_space.chip))
+            return self.__last_chip_space
 
         except StopIteration:
             raise PacmanPlaceException(  # pylint: disable=raise-missing-from

From cb005ceb550219ec74b55c397b063ee2ccde3ac2 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 10:07:01 +0000
Subject: [PATCH 13/33] remove unused import

---
 pacman/operations/placer_algorithms/application_placer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index d7a883c77..b5bf239a8 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -16,7 +16,7 @@
 import os
 from typing import Iterable, List, Optional, Set, Tuple
 
-from spinn_utilities.config_holder import get_config_bool, get_config_str
+from spinn_utilities.config_holder import get_config_bool
 from spinn_utilities.log import FormatAdapter
 from spinn_utilities.ordered_set import OrderedSet
 from spinn_utilities.progress_bar import ProgressBar

From 2379d978a1687b60adf7193505d3b45d7e6ca9fe Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 10:14:32 +0000
Subject: [PATCH 14/33] seperate two functions properly

---
 pacman/operations/placer_algorithms/application_placer.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index b5bf239a8..07a1b6b18 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -92,8 +92,8 @@ def _place_vertex(
         chips_attempted = list()
         try:
             # Start a new space
-            next_chip_space, space = spaces.get_next_chip()
-            space = _Space(next_chip_space)
+            next_chip_space = spaces.get_next_chip()
+            space = _Space(next_chip_space.chip)
             logger.debug(f"Starting placement from {next_chip_space}")
 
             placements_to_make: List = list()

From 5e7887c90e83dabdcc16473c34ea773f8a3a034a Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 11:54:24 +0000
Subject: [PATCH 15/33] space.has_next()

---
 .../placer_algorithms/application_placer.py           | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 07a1b6b18..398ca2626 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -448,7 +448,7 @@ def get_next_chip_space(
             space.update(self.__usable_from_chip(last_chip))
 
         # If no space, error
-        if not space:
+        if not space.has_next():
             self.__last_chip_space = None
             raise _SpaceExceededException(
                 "No more chips to place on in this space; "
@@ -471,8 +471,10 @@ def n_chips_used(self) -> int:
 
     def __usable_from_chip(self, chip: Chip) -> Iterable[Chip]:
         """
+        Get the unused Chips linked to from this Chip
+
         :param Chip chip:
-        :rtype set(Chip)
+        :rtype iterable(Chip)
         """
         for link in chip.router.links:
             target = self.__machine[link.destination_x, link.destination_y]
@@ -505,8 +507,9 @@ def __init__(self, chip: Chip):
         self.__same_board_chips: OrderedSet[Chip] = OrderedSet()
         self.__remaining_chips: OrderedSet[Chip] = OrderedSet()
 
-    def __len__(self) -> int:
-        return len(self.__same_board_chips) + len(self.__remaining_chips)
+    def has_next(self):
+        return (len(self.__same_board_chips) > 0 or
+                len(self.__remaining_chips) > 0)
 
     def __on_same_board(self, chip: Chip) -> bool:
         return (chip.nearest_ethernet_x == self.__board_x and

From 7b16b83a942fcb1823d4e7c1a9b1827b3352a36e Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 12:25:36 +0000
Subject: [PATCH 16/33] store Chips attempted in _Spaces

---
 .../placer_algorithms/application_placer.py   | 31 +++++++++++++------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 398ca2626..c84f3699d 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -89,7 +89,6 @@ def _place_vertex(
     # Try placements from the next chip, but try again if fails
     placed = False
     while not placed:
-        chips_attempted = list()
         try:
             # Start a new space
             next_chip_space = spaces.get_next_chip()
@@ -119,7 +118,7 @@ def _place_vertex(
 
                 # If this worked, store placements to be made
                 last_chip_space = next_chip_space
-                chips_attempted.append(next_chip_space.chip)
+                spaces.save_chip(next_chip_space.chip)
                 _store_on_chip(
                     placements_to_make, vertices_to_place, actual_sdram,
                     next_chip_space)
@@ -127,16 +126,15 @@ def _place_vertex(
             # Now make the placements having confirmed all can be done
             placements.add_placements(placements_to_make)
             placed = True
-            logger.debug(f"Used {chips_attempted}")
+            logger.debug(f"Used {spaces.chips_saved()}")
+            spaces.clear_saved()
         except _SpaceExceededException:
             # This might happen while exploring a space; this may not be
             # fatal since the last space might have just been bound by
             # existing placements, and there might be bigger spaces out
             # there to use
             _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
-            logger.debug(f"Failed, saving {chips_attempted}")
-            spaces.save_chips(chips_attempted)
-            chips_attempted.clear()
+            logger.debug(f"Failed, saving {spaces.chips_saved()}")
 
 
 def _place_error(
@@ -481,11 +479,20 @@ def __usable_from_chip(self, chip: Chip) -> Iterable[Chip]:
             if target not in self.__used_chips:
                 yield target
 
-    def save_chips(self, chips: Iterable[Chip]):
+    def save_chip(self, chip: Chip):
         """
-        :param iterable(Chip) chips:
+        Marks a Chip as to be used by the current Vertex
+        :param Chip chip:
+        """
+        self.__saved_chips.add(chip)
+
+    def chips_saved(self):
+        """
+        The chips saved but not yet permanently used
+
+        :rtype:int
         """
-        self.__saved_chips.update(chips)
+        return self.__saved_chips
 
     def restore_chips(self) -> None:
         """
@@ -496,6 +503,12 @@ def restore_chips(self) -> None:
             self.__restored_chips.add(chip)
         self.__saved_chips.clear()
 
+    def clear_saved(self) -> None:
+        """
+        Clears the saved Chip which makes the permanently used
+        """
+        self.__saved_chips.clear()
+
 
 class _Space(object):
     __slots__ = ("__same_board_chips", "__remaining_chips",

From 357c12072c62d26f5ae171f3389c239408b46f7c Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 15:41:32 +0000
Subject: [PATCH 17/33] split out get possible placements

---
 .../placer_algorithms/application_placer.py   | 103 ++++++++++--------
 1 file changed, 55 insertions(+), 48 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index c84f3699d..3bc52a27f 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -86,55 +86,63 @@ def _place_vertex(
         # This vertex does not require placement or delegates
         return
 
-    # Try placements from the next chip, but try again if fails
-    placed = False
-    while not placed:
-        try:
-            # Start a new space
-            next_chip_space = spaces.get_next_chip()
-            space = _Space(next_chip_space.chip)
-            logger.debug(f"Starting placement from {next_chip_space}")
+    while True:
+        placements_to_make = _prepare_placements(
+            spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex)
+        if placements_to_make is not None:
+            break
+    
+    # Now make the placements having confirmed all can be done
+    placements.add_placements(placements_to_make)
+    logger.debug(f"Used {spaces.chips_saved()}")
+    spaces.clear_saved()
 
-            placements_to_make: List = list()
 
-            # Go through the groups
-            last_chip_space: Optional[_ChipWithSpace] = None
-            for vertices, sdram in same_chip_groups:
-                vertices_to_place = [
-                    vertex
-                    for vertex in vertices
-                    # No need to place virtual vertices
-                    if not isinstance(vertex, AbstractVirtual)
-                    and not placements.is_vertex_placed(vertex)]
-                actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
-                n_cores = len(vertices_to_place)
-
-                # Try to find a chip with space; this might result in a
-                # _SpaceExceededException
-                while not next_chip_space.is_space(n_cores, actual_sdram):
-                    next_chip_space = spaces.get_next_chip_space(
-                        space, last_chip_space)
-                    last_chip_space = None
-
-                # If this worked, store placements to be made
-                last_chip_space = next_chip_space
-                spaces.save_chip(next_chip_space.chip)
-                _store_on_chip(
-                    placements_to_make, vertices_to_place, actual_sdram,
-                    next_chip_space)
-
-            # Now make the placements having confirmed all can be done
-            placements.add_placements(placements_to_make)
-            placed = True
-            logger.debug(f"Used {spaces.chips_saved()}")
-            spaces.clear_saved()
-        except _SpaceExceededException:
-            # This might happen while exploring a space; this may not be
-            # fatal since the last space might have just been bound by
-            # existing placements, and there might be bigger spaces out
-            # there to use
-            _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
-            logger.debug(f"Failed, saving {spaces.chips_saved()}")
+def _prepare_placements(
+        spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex):
+    try:
+        # Start a new space
+        next_chip_space = spaces.get_next_chip()
+        space = _Space(next_chip_space.chip)
+        logger.debug(f"Starting placement from {next_chip_space}")
+
+        placements_to_make: List = list()
+
+        # Go through the groups
+        last_chip_space: Optional[_ChipWithSpace] = None
+        for vertices, sdram in same_chip_groups:
+            vertices_to_place = [
+                vertex
+                for vertex in vertices
+                # No need to place virtual vertices
+                if not isinstance(vertex, AbstractVirtual)
+                and not placements.is_vertex_placed(vertex)]
+            actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
+            n_cores = len(vertices_to_place)
+
+            # Try to find a chip with space; this might result in a
+            # _SpaceExceededException
+            while not next_chip_space.is_space(n_cores, actual_sdram):
+                next_chip_space = spaces.get_next_chip_space(
+                    space, last_chip_space)
+                last_chip_space = None
+
+            # If this worked, store placements to be made
+            last_chip_space = next_chip_space
+            spaces.save_chip(next_chip_space.chip)
+            _store_on_chip(
+                placements_to_make, vertices_to_place, actual_sdram,
+                next_chip_space)
+        return placements_to_make
+
+    except _SpaceExceededException:
+        # This might happen while exploring a space; this may not be
+        # fatal since the last space might have just been bound by
+        # existing placements, and there might be bigger spaces out
+        # there to use
+        _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
+        logger.debug(f"Failed, saving {spaces.chips_saved()}")
+        return None
 
 
 def _place_error(
@@ -340,7 +348,6 @@ def _do_fixed_location(
                 # pylint: disable=raise-missing-from
                 raise PacmanConfigurationException(
                     f"No more cores available on {x}, {y}: {on_chip}")
-        placements.add_placement(Placement(vertex, x, y, next_core))
 
 
 def _store_on_chip(

From ea3d910fb7f2ad97cda2346eeff5c710727c924c Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Tue, 19 Mar 2024 15:42:46 +0000
Subject: [PATCH 18/33] move add_placement to the right place

---
 pacman/operations/placer_algorithms/application_placer.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 07a1b6b18..6f716dafe 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -337,12 +337,11 @@ def _do_fixed_location(
         fixed = vertex.get_fixed_location()
         if not fixed or fixed.p is None:
             try:
-                next_core = next(next_cores)
+                placements.add_placement(Placement(vertex, x, y, next(next_cores)))
             except StopIteration:
                 # pylint: disable=raise-missing-from
                 raise PacmanConfigurationException(
                     f"No more cores available on {x}, {y}: {on_chip}")
-        placements.add_placement(Placement(vertex, x, y, next_core))
 
 
 def _store_on_chip(

From ded6573b62b5e382450d07e8891e06c73d5754d2 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 07:48:38 +0000
Subject: [PATCH 19/33] _filter_vertices method

---
 .../placer_algorithms/application_placer.py   | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 1e1e84f50..f9b098f38 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -111,12 +111,7 @@ def _prepare_placements(
         # Go through the groups
         last_chip_space: Optional[_ChipWithSpace] = None
         for vertices, sdram in same_chip_groups:
-            vertices_to_place = [
-                vertex
-                for vertex in vertices
-                # No need to place virtual vertices
-                if not isinstance(vertex, AbstractVirtual)
-                and not placements.is_vertex_placed(vertex)]
+            vertices_to_place = _filter_vertices(vertices, placements)
             actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
             n_cores = len(vertices_to_place)
 
@@ -145,6 +140,21 @@ def _prepare_placements(
         return None
 
 
+def _filter_vertices(vertices, placements):
+    # Remove any already placed
+    vertices_to_place = [
+        vertex for vertex in vertices
+        if not placements.is_vertex_placed(vertex)]
+    if len(vertices_to_place) != len(vertices) and len(vertices_to_place) > 0:
+        # Putting the rest on a different chip is wrong
+        # Putting them on the same chip is hard so will do if needed
+        raise NotImplementedError(
+            "Unexpected mix of placed and unplaced vertices")
+    # No need to place virtual vertices
+    return [vertex for vertex in vertices_to_place
+            if not isinstance(vertex, AbstractVirtual)]
+
+
 def _place_error(
         placements: Placements, system_placements: Placements,
         exception: Exception,
@@ -294,12 +304,7 @@ def _place_fixed_vertex(
         raise NotImplementedError("Unexpected mix of Fixed and no groups")
 
     for vertices, _ in same_chip_groups:
-        vertices_to_place = [
-            vertex for vertex in vertices
-            # No need to place virtual vertices
-            if not isinstance(vertex, AbstractVirtual)
-            and not placements.is_vertex_placed(vertex)]
-
+        vertices_to_place = _filter_vertices(vertices, placements)
         _do_fixed_location(vertices_to_place, placements)
 
 
@@ -317,6 +322,8 @@ def _do_fixed_location(
             x, y = loc.x, loc.y
             break
     else:
+        # Mixing fixed and free allocations while still keeping the whole
+        # App vertex together is hard so will only do is needed
         raise NotImplementedError(
             "Mixing fixed location and not fixed location groups "
             "within one vertex")

From addc8af0ae5bf078b226cbeb8ec2084056d95638 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 08:14:20 +0000
Subject: [PATCH 20/33] return None rather than raise an Exception in no more
 chips

---
 .../placer_algorithms/application_placer.py   | 94 +++++++++----------
 1 file changed, 44 insertions(+), 50 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index f9b098f38..88dfc450e 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -94,50 +94,44 @@ def _place_vertex(
     
     # Now make the placements having confirmed all can be done
     placements.add_placements(placements_to_make)
-    logger.debug(f"Used {spaces.chips_saved()}")
+    logger.debug("Used {}", spaces.chips_saved())
     spaces.clear_saved()
 
 
 def _prepare_placements(
         spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex):
-    try:
-        # Start a new space
-        next_chip_space = spaces.get_next_chip()
-        space = _Space(next_chip_space.chip)
-        logger.debug(f"Starting placement from {next_chip_space}")
-
-        placements_to_make: List = list()
-
-        # Go through the groups
-        last_chip_space: Optional[_ChipWithSpace] = None
-        for vertices, sdram in same_chip_groups:
-            vertices_to_place = _filter_vertices(vertices, placements)
-            actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
-            n_cores = len(vertices_to_place)
-
-            # Try to find a chip with space; this might result in a
-            # _SpaceExceededException
-            while not next_chip_space.is_space(n_cores, actual_sdram):
-                next_chip_space = spaces.get_next_chip_space(
-                    space, last_chip_space)
-                last_chip_space = None
-
-            # If this worked, store placements to be made
-            last_chip_space = next_chip_space
-            spaces.save_chip(next_chip_space.chip)
-            _store_on_chip(
-                placements_to_make, vertices_to_place, actual_sdram,
-                next_chip_space)
-        return placements_to_make
-
-    except _SpaceExceededException:
-        # This might happen while exploring a space; this may not be
-        # fatal since the last space might have just been bound by
-        # existing placements, and there might be bigger spaces out
-        # there to use
-        _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
-        logger.debug(f"Failed, saving {spaces.chips_saved()}")
-        return None
+    # Start a new space
+    next_chip_space = spaces.get_next_chip()
+    space = _Space(next_chip_space.chip)
+    logger.debug("Starting placement from {}", next_chip_space)
+
+    placements_to_make: List = list()
+
+    # Go through the groups
+    last_chip_space: Optional[_ChipWithSpace] = None
+    for vertices, sdram in same_chip_groups:
+        vertices_to_place = _filter_vertices(vertices, placements)
+        actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
+        n_cores = len(vertices_to_place)
+
+        # Try to find a chip with space
+        while not next_chip_space.is_space(n_cores, actual_sdram):
+            next_chip_space = spaces.get_next_chip_space(
+                space, last_chip_space)
+            if next_chip_space is None:
+                # Ran out of space so need try a new start chip
+                _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
+                logger.debug("Failed, saving {}", spaces.chips_saved())
+                return None
+            last_chip_space = None
+
+        # If this worked, store placements to be made
+        last_chip_space = next_chip_space
+        spaces.save_chip(next_chip_space.chip)
+        _store_on_chip(
+            placements_to_make, vertices_to_place, actual_sdram,
+            next_chip_space)
+    return placements_to_make
 
 
 def _filter_vertices(vertices, placements):
@@ -293,10 +287,6 @@ def _check_could_fit(
     raise PacmanTooBigToPlace(message)
 
 
-class _SpaceExceededException(Exception):
-    pass
-
-
 def _place_fixed_vertex(
         app_vertex: ApplicationVertex, placements: Placements):
     same_chip_groups = app_vertex.splitter.get_same_chip_groups()
@@ -446,13 +436,16 @@ def __get_next_chip(self) -> Chip:
         return self.__next_chip
 
     def get_next_chip_space(
-            self, space: _Space,
-            last_chip_space: Optional[_ChipWithSpace]) -> _ChipWithSpace:
+            self, space: _Space,  last_chip_space: Optional[_ChipWithSpace]
+            ) -> Optional[_ChipWithSpace]:
         """
+        Gets the next neighbouring Chip and its space
+
+        If No more Chips available returns None
+
         :param _Space space:
         :param _ChipWithSpace last_chip_space:
-        :rtype: _ChipWithSpace
-        :raises _SpaceExceededException:
+        :rtype: _ChipWithSpace or None
         """
         # If we are reporting a used chip, update with reachable chips
         if last_chip_space is not None:
@@ -462,9 +455,10 @@ def get_next_chip_space(
         # If no space, error
         if not space.has_next():
             self.__last_chip_space = None
-            raise _SpaceExceededException(
-                "No more chips to place on in this space; "
-                f"{self.n_chips_used} of {self.__machine.n_chips} used")
+            logger.debug("No more chips to place on in this space; "
+                         "{} of {} used",
+                         self.n_chips_used, self.__machine.n_chips)
+            return None
         chip = space.pop()
         self.__used_chips.add(chip)
         self.__restored_chips.discard(chip)

From 777be64a42d038983539f29091b44fc8365745ee Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 09:21:30 +0000
Subject: [PATCH 21/33] add tests

---
 .../test_application_placer.py                | 56 ++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index 04a4a5a66..857b3ebb9 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -14,7 +14,8 @@
 from spinn_utilities.config_holder import set_config
 from spinn_machine.virtual_machine import virtual_machine
 from pacman.data.pacman_data_writer import PacmanDataWriter
-from pacman.exceptions import PacmanTooBigToPlace
+from pacman.exceptions import (
+    PacmanConfigurationException, PacmanPlaceException, PacmanTooBigToPlace)
 from pacman.model.partitioner_splitters import (
     SplitterFixedLegacy, AbstractSplitterCommon)
 from pacman.operations.placer_algorithms.application_placer import (
@@ -107,6 +108,59 @@ def test_application_placer():
     place_application_graph(Placements())
 
 
+def test_application_placer_large_groups():
+    unittest_setup()
+    set_config("Machine", "version", 5)
+    writer = PacmanDataWriter.mock()
+    # fixed early works as this vertex is looked at first
+    fixed = SimpleTestVertex(10, "FIXED", max_atoms_per_core=1)
+    fixed.splitter = SplitterFixedLegacy()
+    fixed.set_fixed_location(0, 0)
+    writer.add_vertex(fixed)
+    fixed.splitter.create_machine_vertices(ChipCounter())
+    for i in range(17):
+        _make_vertices(writer, 1000, 14, 17, f"app_vertex_{i}")
+    writer.set_machine(virtual_machine(24, 12))
+    place_application_graph(Placements())
+
+
+def test_application_placer_too_few_boards():
+    unittest_setup()
+    set_config("Machine", "version", 5)
+    writer = PacmanDataWriter.mock()
+    # fixed early works as this vertex is looked at first
+    fixed = SimpleTestVertex(10, "FIXED", max_atoms_per_core=1)
+    fixed.splitter = SplitterFixedLegacy()
+    fixed.set_fixed_location(0, 0)
+    writer.add_vertex(fixed)
+    fixed.splitter.create_machine_vertices(ChipCounter())
+    for i in range(56):
+        _make_vertices(writer, 1000, 14, 5, f"app_vertex_{i}")
+    writer.set_machine(virtual_machine(12, 12))
+    try:
+        place_application_graph(Placements())
+        raise AssertionError("Error not raise")
+    except PacmanPlaceException as ex:
+        assert ("No more chips to place" in str(ex))
+
+
+def test_application_placer_restart_needed():
+    unittest_setup()
+    set_config("Machine", "version", 5)
+    writer = PacmanDataWriter.mock()
+    for (x, y) in [(1, 0), (1, 1), (0, 1)]:
+        fixed = SimpleTestVertex(15, f"FIXED {x}:{y}", max_atoms_per_core=1)
+        fixed.splitter = SplitterFixedLegacy()
+        fixed.set_fixed_location(x, y)
+        writer.add_vertex(fixed)
+        fixed.splitter.create_machine_vertices(ChipCounter())
+    for i in range(56):
+        _make_vertices(writer, 1000, 14, 5, f"app_vertex_{i}")
+    # Don't use a full wrap machine
+    writer.set_machine(virtual_machine(28, 16))
+    place_application_graph(Placements())
+
+
 def test_application_placer_late_fixed():
     unittest_setup()
     set_config("Machine", "version", 5)

From 004a030566017229def8878010a8c7099948cd19 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 10:21:11 +0000
Subject: [PATCH 22/33] ensure Chips are in order (they are not in Virtual
 Machines)

---
 .../placer_algorithms/application_placer.py       | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 6f716dafe..0aa095997 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -374,13 +374,26 @@ def __init__(
         self.__machine = PacmanDataView.get_machine()
         self.__placements = placements
         self.__plan_n_timesteps = plan_n_timesteps
-        self.__chips = self.__machine.chips
+        self.__chips = self.__chip_order()
         self.__next_chip = next(self.__chips)
         self.__used_chips: Set[Chip] = set()
         self.__last_chip_space: Optional[_ChipWithSpace] = None
         self.__saved_chips: OrderedSet[Chip] = OrderedSet()
         self.__restored_chips: OrderedSet[Chip] = OrderedSet()
 
+    def __chip_order(self):
+        """
+        Iterate the Chips in a guaranteed order
+
+        :param Machine machine:
+        :rtype: iterable(Chip)
+        """
+        for x in range(self.__machine.width):
+            for y in range(self.__machine.height):
+                chip = self.__machine.get_chip_at(x, y)
+                if chip:
+                    yield chip
+
     def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
         """
         Gets the core ids and the plan sdram of the placements for this Chip

From 7ddc40985c367dea24a963b78017e50ccb067f8c Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 12:51:53 +0000
Subject: [PATCH 23/33] move _Space inside _Spaces

---
 .../placer_algorithms/application_placer.py    | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 7fcdcef53..7cd005aaa 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -102,7 +102,6 @@ def _prepare_placements(
         spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex):
     # Start a new space
     next_chip_space = spaces.get_next_chip()
-    space = _Space(next_chip_space.chip)
     logger.debug("Starting placement from {}", next_chip_space)
 
     placements_to_make: List = list()
@@ -116,8 +115,7 @@ def _prepare_placements(
 
         # Try to find a chip with space
         while not next_chip_space.is_space(n_cores, actual_sdram):
-            next_chip_space = spaces.get_next_chip_space(
-                space, last_chip_space)
+            next_chip_space = spaces.get_next_chip_space(last_chip_space)
             if next_chip_space is None:
                 # Ran out of space so need try a new start chip
                 _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
@@ -366,7 +364,8 @@ def _store_on_chip(
 class _Spaces(object):
     __slots__ = ("__machine", "__chips", "__next_chip", "__used_chips",
                  "__system_placements", "__placements", "__plan_n_timesteps",
-                 "__last_chip_space", "__saved_chips", "__restored_chips")
+                 "__last_chip_space", "__saved_chips", "__restored_chips",
+                 "__space")
 
     def __init__(
             self, placements: Placements, plan_n_timesteps: Optional[int]):
@@ -383,6 +382,7 @@ def __init__(
         self.__last_chip_space: Optional[_ChipWithSpace] = None
         self.__saved_chips: OrderedSet[Chip] = OrderedSet()
         self.__restored_chips: OrderedSet[Chip] = OrderedSet()
+        self.__space = _Space(self.__next_chip)
 
     def __chip_order(self):
         """
@@ -424,6 +424,7 @@ def get_next_chip(self) -> _ChipWithSpace:
                     chip, cores_used, sdram_used)
                 self.__used_chips.add(chip)
 
+            self.__space = _Space(self.__last_chip_space.chip)
             return self.__last_chip_space
 
         except StopIteration:
@@ -449,30 +450,29 @@ def __get_next_chip(self) -> Chip:
         return self.__next_chip
 
     def get_next_chip_space(
-            self, space: _Space,  last_chip_space: Optional[_ChipWithSpace]
+            self, last_chip_space: Optional[_ChipWithSpace]
             ) -> Optional[_ChipWithSpace]:
         """
         Gets the next neighbouring Chip and its space
 
         If No more Chips available returns None
 
-        :param _Space space:
         :param _ChipWithSpace last_chip_space:
         :rtype: _ChipWithSpace or None
         """
         # If we are reporting a used chip, update with reachable chips
         if last_chip_space is not None:
             last_chip = last_chip_space.chip
-            space.update(self.__usable_from_chip(last_chip))
+            self.__space.update(self.__usable_from_chip(last_chip))
 
         # If no space, error
-        if not space.has_next():
+        if not self.__space.has_next():
             self.__last_chip_space = None
             logger.debug("No more chips to place on in this space; "
                          "{} of {} used",
                          self.n_chips_used, self.__machine.n_chips)
             return None
-        chip = space.pop()
+        chip = self.__space.pop()
         self.__used_chips.add(chip)
         self.__restored_chips.discard(chip)
         cores_used, sdram_used = self.__cores_and_sdram(chip)

From fd9155a593fc02fd84238a60c28df46898210ed2 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Wed, 20 Mar 2024 16:05:02 +0000
Subject: [PATCH 24/33] Replace OrderedSet

---
 .../placer_algorithms/application_placer.py   | 132 ++++++++----------
 1 file changed, 58 insertions(+), 74 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 7cd005aaa..1a10b1d72 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -14,11 +14,10 @@
 from __future__ import annotations
 import logging
 import os
-from typing import Iterable, List, Optional, Set, Tuple
+from typing import Dict, List, Optional, Set, Tuple
 
 from spinn_utilities.config_holder import get_config_bool
 from spinn_utilities.log import FormatAdapter
-from spinn_utilities.ordered_set import OrderedSet
 from spinn_utilities.progress_bar import ProgressBar
 
 from spinn_machine import Chip
@@ -365,7 +364,8 @@ class _Spaces(object):
     __slots__ = ("__machine", "__chips", "__next_chip", "__used_chips",
                  "__system_placements", "__placements", "__plan_n_timesteps",
                  "__last_chip_space", "__saved_chips", "__restored_chips",
-                 "__space")
+                 "__ethernet_x", "__ethernet_y", "__same_board_chips",
+                 "__other_board_chips")
 
     def __init__(
             self, placements: Placements, plan_n_timesteps: Optional[int]):
@@ -380,9 +380,14 @@ def __init__(
         self.__next_chip = next(self.__chips)
         self.__used_chips: Set[Chip] = set()
         self.__last_chip_space: Optional[_ChipWithSpace] = None
-        self.__saved_chips: OrderedSet[Chip] = OrderedSet()
-        self.__restored_chips: OrderedSet[Chip] = OrderedSet()
-        self.__space = _Space(self.__next_chip)
+        self.__saved_chips: List[Chip] = list()
+        self.__restored_chips: List[Chip] = list()
+
+        # Set some value so no Optional needed
+        self.__ethernet_x = self.__next_chip.nearest_ethernet_x
+        self.__ethernet_y = self.__next_chip.nearest_ethernet_y
+        self.__same_board_chips: Dict[Chip, Chip] = dict()
+        self.__other_board_chips:  Dict[Chip, Chip] = dict()
 
     def __chip_order(self):
         """
@@ -424,7 +429,12 @@ def get_next_chip(self) -> _ChipWithSpace:
                     chip, cores_used, sdram_used)
                 self.__used_chips.add(chip)
 
-            self.__space = _Space(self.__last_chip_space.chip)
+            chip = self.__last_chip_space.chip
+            # reset the neighbour chip info
+            self.__ethernet_x = chip.nearest_ethernet_x
+            self.__ethernet_y = chip.nearest_ethernet_y
+            self.__same_board_chips.clear()
+            self.__other_board_chips.clear()
             return self.__last_chip_space
 
         except StopIteration:
@@ -442,7 +452,7 @@ def __get_next_chip(self) -> Chip:
         :raises: StopIteration
         """
         while self.__restored_chips:
-            chip = self.__restored_chips.pop(last=False)
+            chip = self.__restored_chips.pop[0]
             if chip not in self.__used_chips:
                 return chip
         while self.__next_chip in self.__used_chips:
@@ -463,18 +473,19 @@ def get_next_chip_space(
         # If we are reporting a used chip, update with reachable chips
         if last_chip_space is not None:
             last_chip = last_chip_space.chip
-            self.__space.update(self.__usable_from_chip(last_chip))
+            self.__add_neighbours(last_chip)
 
         # If no space, error
-        if not self.__space.has_next():
+        if not self.__has_neighbour():
             self.__last_chip_space = None
             logger.debug("No more chips to place on in this space; "
                          "{} of {} used",
                          self.n_chips_used, self.__machine.n_chips)
             return None
-        chip = self.__space.pop()
+        chip = self.__pop_neighbour()
         self.__used_chips.add(chip)
-        self.__restored_chips.discard(chip)
+        if chip in self.__restored_chips:
+            self.__restored_chips.remove(chip)
         cores_used, sdram_used = self.__cores_and_sdram(chip)
         self.__last_chip_space = _ChipWithSpace(chip, cores_used, sdram_used)
         return self.__last_chip_space
@@ -488,30 +499,53 @@ def n_chips_used(self) -> int:
         """
         return len(self.__used_chips)
 
-    def __usable_from_chip(self, chip: Chip) -> Iterable[Chip]:
-        """
-        Get the unused Chips linked to from this Chip
-
-        :param Chip chip:
-        :rtype iterable(Chip)
-        """
+    def __add_neighbours(self, chip: Chip):
         for link in chip.router.links:
             target = self.__machine[link.destination_x, link.destination_y]
             if target not in self.__used_chips:
-                yield target
+                if (target.nearest_ethernet_x == self.__ethernet_x and
+                        target.nearest_ethernet_y == self.__ethernet_y):
+                    self.__same_board_chips[target] = target
+                else:
+                    self.__other_board_chips[target] = target
+
+    def __has_neighbour(self):
+        return (len(self.__same_board_chips) > 0 or
+                len(self.__other_board_chips) > 0)
+
+    def __pop_neighbour(self):
+        if self.__same_board_chips:
+            k = next(iter(self.__same_board_chips))
+            del self.__same_board_chips[k]
+            return k
+        if self.__other_board_chips:
+            next_chip = next(iter(self.__other_board_chips))
+            del self.__other_board_chips[next_chip]
+            self.__ethernet_x = next_chip.nearest_ethernet_x
+            self.__ethernet_y = next_chip.nearest_ethernet_y
+            to_check = list(self.__other_board_chips)
+            self.__other_board_chips.clear()
+            for chip in to_check:
+                if (chip.nearest_ethernet_x == self.__ethernet_x and
+                        chip.nearest_ethernet_y == self.__ethernet_y):
+                    self.__same_board_chips[chip] = chip
+                else:
+                    self.__other_board_chips[chip] = chip
+            return next_chip
+        raise StopIteration
 
     def save_chip(self, chip: Chip):
         """
         Marks a Chip as to be used by the current Vertex
         :param Chip chip:
         """
-        self.__saved_chips.add(chip)
+        self.__saved_chips.append(chip)
 
-    def chips_saved(self):
+    def chips_saved(self) -> List[Chip]:
         """
         The chips saved but not yet permanently used
 
-        :rtype:int
+        :rtype:list(Chip)
         """
         return self.__saved_chips
 
@@ -521,7 +555,7 @@ def restore_chips(self) -> None:
         """
         for chip in self.__saved_chips:
             self.__used_chips.remove(chip)
-            self.__restored_chips.add(chip)
+            self.__restored_chips.append(chip)
         self.__saved_chips.clear()
 
     def clear_saved(self) -> None:
@@ -531,56 +565,6 @@ def clear_saved(self) -> None:
         self.__saved_chips.clear()
 
 
-class _Space(object):
-    __slots__ = ("__same_board_chips", "__remaining_chips",
-                 "__board_x", "__board_y", "__first_chip")
-
-    def __init__(self, chip: Chip):
-        self.__board_x = chip.nearest_ethernet_x
-        self.__board_y = chip.nearest_ethernet_y
-        self.__same_board_chips: OrderedSet[Chip] = OrderedSet()
-        self.__remaining_chips: OrderedSet[Chip] = OrderedSet()
-
-    def has_next(self):
-        return (len(self.__same_board_chips) > 0 or
-                len(self.__remaining_chips) > 0)
-
-    def __on_same_board(self, chip: Chip) -> bool:
-        return (chip.nearest_ethernet_x == self.__board_x and
-                chip.nearest_ethernet_y == self.__board_y)
-
-    def pop(self) -> Chip:
-        """
-        :rtype: Chip
-        :raise: StopIteration
-        """
-        if self.__same_board_chips:
-            return self.__same_board_chips.pop(last=False)
-        if self.__remaining_chips:
-            next_chip = self.__remaining_chips.pop(last=False)
-            self.__board_x = next_chip.nearest_ethernet_x
-            self.__board_y = next_chip.nearest_ethernet_y
-            to_remove = list()
-            for chip in self.__remaining_chips:
-                if self.__on_same_board(chip):
-                    to_remove.append(chip)
-                    self.__same_board_chips.add(chip)
-            for chip in to_remove:
-                self.__remaining_chips.remove(chip)
-            return next_chip
-        raise StopIteration
-
-    def update(self, chips: Iterable[Chip]):
-        """
-        :param iterable(Chip) chips:
-        """
-        for chip in chips:
-            if self.__on_same_board(chip):
-                self.__same_board_chips.add(chip)
-            else:
-                self.__remaining_chips.add(chip)
-
-
 class _ChipWithSpace(object):
     """
     A chip with space for placement.

From ad4e3a687d00ef7b2c56967634339804262e6165 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 12:49:32 +0000
Subject: [PATCH 25/33] refactor application placer

---
 .../placer_algorithms/application_placer.py   | 534 ++++++++++--------
 .../test_application_placer.py                |  15 +-
 2 files changed, 315 insertions(+), 234 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 1a10b1d72..9fbd16da1 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -58,7 +58,7 @@ def place_application_graph(system_placements: Placements) -> Placements:
                 _place_fixed_vertex(app_vertex, placements)
 
         plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
-        spaces = _Spaces(placements, plan_n_timesteps)
+        spaces = _Spaces(placements)
         for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
             # as this checks if placed already not need to check if fixed
             _place_vertex(app_vertex, spaces, plan_n_timesteps, placements)
@@ -78,56 +78,46 @@ def place_application_graph(system_placements: Placements) -> Placements:
 def _place_vertex(
         app_vertex: ApplicationVertex, spaces: _Spaces,
         plan_n_timesteps: Optional[int], placements: Placements):
-    spaces.restore_chips()
-
     same_chip_groups = app_vertex.splitter.get_same_chip_groups()
     if not same_chip_groups:
         # This vertex does not require placement or delegates
         return
 
+    spaces.start_app_vertex(app_vertex)
+    # try to make placements with a different start Chip each time
     while True:
         placements_to_make = _prepare_placements(
-            spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex)
+            spaces, placements, plan_n_timesteps, same_chip_groups)
         if placements_to_make is not None:
             break
     
-    # Now make the placements having confirmed all can be done
+    # Now actually add the placements having confirmed all can be done
     placements.add_placements(placements_to_make)
-    logger.debug("Used {}", spaces.chips_saved())
-    spaces.clear_saved()
 
 
 def _prepare_placements(
-        spaces, placements, plan_n_timesteps, same_chip_groups, app_vertex):
-    # Start a new space
-    next_chip_space = spaces.get_next_chip()
-    logger.debug("Starting placement from {}", next_chip_space)
-
+        spaces, placements, plan_n_timesteps, same_chip_groups):
+    spaces.start_preparing()
     placements_to_make: List = list()
 
     # Go through the groups
-    last_chip_space: Optional[_ChipWithSpace] = None
     for vertices, sdram in same_chip_groups:
         vertices_to_place = _filter_vertices(vertices, placements)
-        actual_sdram = sdram.get_total_sdram(plan_n_timesteps)
+        if len(vertices_to_place) == 0:
+            continue
+        plan_sdram = sdram.get_total_sdram(plan_n_timesteps)
         n_cores = len(vertices_to_place)
 
         # Try to find a chip with space
-        while not next_chip_space.is_space(n_cores, actual_sdram):
-            next_chip_space = spaces.get_next_chip_space(last_chip_space)
-            if next_chip_space is None:
-                # Ran out of space so need try a new start chip
-                _check_could_fit(app_vertex, vertices_to_place, actual_sdram)
-                logger.debug("Failed, saving {}", spaces.chips_saved())
-                return None
-            last_chip_space = None
+        chip = spaces.get_next_chip_with_space(n_cores, plan_sdram)
+        if chip is None:
+            return None
 
         # If this worked, store placements to be made
-        last_chip_space = next_chip_space
-        spaces.save_chip(next_chip_space.chip)
-        _store_on_chip(
-            placements_to_make, vertices_to_place, actual_sdram,
-            next_chip_space)
+        for vertex in vertices:
+            core = spaces.pop_next_core()
+            placements_to_make.append(Placement(
+                vertex, chip.x, chip.y, core))
     return placements_to_make
 
 
@@ -245,16 +235,8 @@ def _check_could_fit(
     :param int sdram:
     :raises PacmanTooBigToPlace:
     """
-    version = PacmanDataView.get_machine_version()
-    max_sdram = (
-            version.max_sdram_per_chip -
-            PacmanDataView.get_all_monitor_sdram().get_total_sdram(
-                PacmanDataView.get_plan_n_timestep()))
-    max_cores = (
-            version.max_cores_per_chip - version.n_scamp_cores -
-            PacmanDataView.get_all_monitor_cores())
     n_cores = len(vertices_to_place)
-    if sdram <= max_sdram and n_cores <= max_cores:
+    if sdram <= self.__max_sdram and n_cores <= self.__max_cores:
         # should fit somewhere
         return
     message = (
@@ -353,42 +335,118 @@ def _store_on_chip(
     :param int sdram:
     :param _ChipWithSpace next_chip_space:
     """
-    for vertex in vertices:
-        core = next_chip_space.use_next_core()
-        placements_to_make.append(Placement(
-            vertex, next_chip_space.x, next_chip_space.y, core))
-    next_chip_space.use_sdram(sdram)
 
 
 class _Spaces(object):
-    __slots__ = ("__machine", "__chips", "__next_chip", "__used_chips",
-                 "__system_placements", "__placements", "__plan_n_timesteps",
-                 "__last_chip_space", "__saved_chips", "__restored_chips",
-                 "__ethernet_x", "__ethernet_y", "__same_board_chips",
-                 "__other_board_chips")
-
-    def __init__(
-            self, placements: Placements, plan_n_timesteps: Optional[int]):
+    __slots__ = (
+        # Values from PacmanDataView cached for speed
+        # PacmanDataView.get_machine()
+        "__machine",
+        # PacmanDataView.get_plan_n_timestep()
+        "__plan_n_timesteps",
+        # Sdram available on perfect none Ethernet Chip after Monitors placed
+        "__max_sdram",
+        # Minumum sdram that should be available for a Chip to not be full
+        "__min_sdram",
+        # N Cores free on perfect none Ethernet Chip after Monitors placed
+        "__max_cores",
+
+        # Pointer to the placements including all previous Application Vertices
+        "__placements",
+        # A Function to yield the Chips in a consistant order
+        "__chips",
+        # Chips that have been fully placed by previous Application Vertices
+        "__full_chips",
+        # Chips that have already been used by this ApplicationVertex
+        "__prepared_chips",
+        # Start Chips from previous ApplicationVertices not yet marked as full
+        "__restored_chips",
+        # Start Chips tried for this ApplicationVertex
+        "__starts_tried",
+        # Label of the current ApplicationVertex for (error) reporting
+        "__app_vertex",
+
+        # Data for the last Chip offered to place on
+        # May be full after current group placed
+        "__current_chip",
+        # List of cores available. Included ones for current group until used
+        "__current_cores_free",
+        # Available sdram after the current group is placed
+        "__current_sdram_free",
+
+        # Data about the neighbouring Chips to ones used
+        # Current board being placed on
+        "__ethernet_x",
+        "__ethernet_y",
+        # List of available neighbours on the current board
+        "__same_board_chips",
+        # List of available neighbours not on the current board
+        "__other_board_chips")
+
+    # __last_chip_space
+    # __used_chip
+    # __saved_chip
+    # __nextChip, next_start
+
+    def __init__(self, placements: Placements):
         """
         :param Placements placements:
         :param int plan_n_timesteps:
         """
+        # Data cached for speed
         self.__machine = PacmanDataView.get_machine()
+        self.__plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
+        version = PacmanDataView.get_machine_version()
+        self.__max_sdram = (
+                version.max_sdram_per_chip -
+                PacmanDataView.get_all_monitor_sdram().get_total_sdram(
+                    PacmanDataView.get_plan_n_timestep()))
+        self.__max_cores = (
+                version.max_cores_per_chip - version.n_scamp_cores -
+                PacmanDataView.get_all_monitor_cores())
+        self.__min_sdram = self.__max_sdram // self.__max_cores
+
         self.__placements = placements
-        self.__plan_n_timesteps = plan_n_timesteps
         self.__chips = self.__chip_order()
-        self.__next_chip = next(self.__chips)
-        self.__used_chips: Set[Chip] = set()
-        self.__last_chip_space: Optional[_ChipWithSpace] = None
-        self.__saved_chips: List[Chip] = list()
+
+        self.__full_chips: Set[Chip] = set()
+        self.__prepared_chips: Set[Chip] = set()
         self.__restored_chips: List[Chip] = list()
+        self.__starts_tried: List[Chip] = list()
+
+        self.__current_chip: Optional[Chip] = None
+        self.__current_cores_free: List[int] = list()
+        self.__current_sdram_free = 0
+        self.__app_vertex = "NO APP VETERX SET"
 
         # Set some value so no Optional needed
-        self.__ethernet_x = self.__next_chip.nearest_ethernet_x
-        self.__ethernet_y = self.__next_chip.nearest_ethernet_y
+        self.__ethernet_x = -1
+        self.__ethernet_y = -1
         self.__same_board_chips: Dict[Chip, Chip] = dict()
         self.__other_board_chips:  Dict[Chip, Chip] = dict()
 
+    def start_app_vertex(self, app_vertex: ApplicationVertex):
+        """
+        Signal that the next Application vertex is starting
+        :param ApplicationVertex app_vertex:
+        """
+        # Store the label for error reporting
+        self.__app_vertex = app_vertex.label
+
+        # Restore the starts tried last time.
+        # Check if they are full comes later
+        while len(self.__starts_tried):
+            self.__restored_chips.append(self.__starts_tried.pop(0))
+        self.start_preparing()
+
+    def start_preparing(self) -> None:
+        """
+        Signal that a new attempt to prepared placements is starting
+        """
+        # Clear the Chips used in the last prepare
+        self.__prepared_chips.clear()
+        self.__current_chip = None
+
     def __chip_order(self):
         """
         Iterate the Chips in a guaranteed order
@@ -402,118 +460,229 @@ def __chip_order(self):
                 if chip:
                     yield chip
 
-    def __cores_and_sdram(self, chip: Chip) -> Tuple[Set[int], int]:
+    def __space_on_chip(
+            self, chip: Chip, n_cores: int, plan_sdram: int) -> bool:
         """
-        Gets the core ids and the plan sdram of the placements for this Chip
+        Checks if the Chip has enough space for this group, Cache if yes
+
+        If the Chip has already full from other Application Vertices,
+        the Chip is added to the full list and False is returned
+
+        If the chip is not full but does not have the space,
+        the Chip is added to the prepared_chips list and False is returned.
+        As safety check is also done to make sure the group could fit on
+        another Chip
+
+        If there there is room on the Chip
+        the Chip is cached and True is returned.
+        The values Cached are the:
+        current_chip Even if full to keep the code simpler
+        current_cores_free Including the ones for this group
+        current_sdram_free Excluding the sdram needed fot this group
 
         :param Chip chip:
-        :return cores, sdram
+        :param int n_cores: number of cores needed
+        :param int plan_sdram:
         :rtype: tuple(int, int)
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
         """
+        cores_free = list(chip.placable_processors_ids)
+        sdram_free = chip.sdram
+
+        # remove the already placed for other Application Vertices
         on_chip = self.__placements.placements_on_chip(chip)
-        cores_used = {p.p for p in on_chip}
-        sdram_used = sum(
-            p.vertex.sdram_required.get_total_sdram(
-                self.__plan_n_timesteps) for p in on_chip)
-        return cores_used, sdram_used
+        if len(on_chip) == len(cores_free):
+            self.__full_chips.add(chip)
+            return False
+
+        for placement in on_chip:
+            cores_free.remove(placement.p)
+            sdram_free -= placement.vertex.sdram_required.get_total_sdram(
+                self.__plan_n_timesteps)
+
+        if sdram_free < self.__min_sdram:
+            self.__full_chips.add(chip)
+            return False
+
+        # Remember this chip so it is not tried again in this preparation
+        # This assumes all groups are the same size so even if too small
+        self.__prepared_chips.add(chip)
+
+        if len(cores_free) < n_cores or sdram_free < plan_sdram:
+            self.__check_could_fit(n_cores, plan_sdram)
+            return False
+
+        # record the current Chip
+        self.__current_chip = chip
+        # cores are popped out later to keep them here for now
+        self.__current_cores_free = cores_free
+        # sdram is the whole group so can be removed now
+        self.__current_sdram_free = sdram_free - plan_sdram
+
+        # adds the neighburs
+        self.__add_neighbours(chip)
+
+        return True
+
+    def __check_could_fit(self, n_cores:int, plan_sdram: int):
+        """
+        :param int n_cores: number of cores needs
+        :param int plan_sdram: minimum amount of SDRAM needed
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
+        """
+        if plan_sdram <= self.__max_sdram and n_cores <= self.__max_cores:
+            # should fit somewhere
+            return
+        message = (
+            f"{self.__app_vertex} will not fit on any possible Chip "
+            f"as a smae_chip_group ")
+
+        version = PacmanDataView.get_machine_version()
+        if plan_sdram > self.__max_sdram:
+            message += f"requires {plan_sdram} bytes but "
+            if plan_sdram > version.max_sdram_per_chip:
+                message += f"a Chip only has {version.max_sdram_per_chip} " \
+                           f"bytes "
+            else:
+                message += f"after monitors only {self.__max_sdram} " \
+                           f"bytes are available "
+            message += "Lowering max_core_per_chip may resolve this."
+            raise PacmanTooBigToPlace(message)
+
+        if n_cores > version.max_cores_per_chip:
+            message += " is more vertices than the number of cores on a chip."
+            raise PacmanTooBigToPlace(message)
+        user_cores = version.max_cores_per_chip - version.n_scamp_cores
+        if n_cores > user_cores:
+            message += (
+                f"is more vertices than the user cores ({user_cores}) "
+                "available on a Chip")
+        else:
+            message += (
+                f"is more vertices than the {self.__max_cores} cores "
+                f"available on a Chip once "
+                f"{PacmanDataView.get_all_monitor_cores()} "
+                f"are reserved for monitors")
+        raise PacmanTooBigToPlace(message)
 
-    def get_next_chip(self) -> _ChipWithSpace:
-        """
-        :rtype: _ChipWithSpace
+    def __get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
         """
-        try:
-            if self.__last_chip_space is None:
-                chip = self.__get_next_chip()
-                cores_used, sdram_used = self.__cores_and_sdram(chip)
-                self.__last_chip_space = _ChipWithSpace(
-                    chip, cores_used, sdram_used)
-                self.__used_chips.add(chip)
-
-            chip = self.__last_chip_space.chip
-            # reset the neighbour chip info
-            self.__ethernet_x = chip.nearest_ethernet_x
-            self.__ethernet_y = chip.nearest_ethernet_y
-            self.__same_board_chips.clear()
-            self.__other_board_chips.clear()
-            return self.__last_chip_space
+        Gets the next start Chip
+        
+        Also sets up the current_chip and starts a new neighbourhood
+         
+        :param int n_cores: number of cores needs 
+        :param int plan_sdram: minimum amount of SDRAM needed
 
-        except StopIteration:
-            raise PacmanPlaceException(  # pylint: disable=raise-missing-from
-                f"No more chips to place on; {self.n_chips_used} of "
-                f"{self.__machine.n_chips} used")
+        :rtype: Chip
+        :raises PacmanPlaceException: If no new start Chip is available
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
+        """
+        # reset the neighbour chip info
+        self.__same_board_chips.clear()
+        self.__other_board_chips.clear()
+
+        # Find the next start chip
+        while True:
+            start = self.__pop_start_chip()
+            # Save as tried as not full even if toO small
+            self.__starts_tried.append(start)
+            # Set the ethernets in case space_on_chip adds neighbours
+            self.__ethernet_x = start.nearest_ethernet_x
+            self.__ethernet_y = start.nearest_ethernet_y
+            if self.__space_on_chip(start, n_cores, plan_sdram):
+                break
+
+        logger.debug("Starting placement from {}", start)
+        return start
 
-    def __get_next_chip(self) -> Chip:
+    def __pop_start_chip(self) -> Chip:
         """
-        Gets the next Chip from either restored or if none the Machine
+        Gets the next start Chip from either restored or if none the Machine
 
-        Ignores any Chip in the used set
+        Ignores any Chip that are already full
 
         :rtype: Chip
-        :raises: StopIteration
+        :raises PacmanPlaceException: If no new start Chip is available
         """
         while self.__restored_chips:
-            chip = self.__restored_chips.pop[0]
-            if chip not in self.__used_chips:
+            chip = self.__restored_chips.pop(0)
+            if chip not in self.__full_chips:
                 return chip
-        while self.__next_chip in self.__used_chips:
-            self.__next_chip = next(self.__chips)
-        return self.__next_chip
+        try:
+            start = next(self.__chips)
+            while start in self.__full_chips:
+                start = next(self.__chips)
+            return start
+        except StopIteration:
+            raise PacmanPlaceException(  # pylint: disable=raise-missing-from
+                f"No more chips to start with for {self.__app_vertex} "
+                f"Out of {self.__machine.n_chips} "
+                f"{len(self.__full_chips)} already full "
+                f"and {len(self.__starts_tried)} tried")
 
-    def get_next_chip_space(
-            self, last_chip_space: Optional[_ChipWithSpace]
-            ) -> Optional[_ChipWithSpace]:
+    def __get_next_neighbour(self, n_cores: int, plan_sdram: int):
         """
-        Gets the next neighbouring Chip and its space
+        Gets the next neighbour Chip
+
+        Also changes the current_chip and updates the neighbourhood
 
-        If No more Chips available returns None
+        This wil return None if there are no more neighbouring Chip big enough
 
-        :param _ChipWithSpace last_chip_space:
-        :rtype: _ChipWithSpace or None
+        :param int n_cores: number of cores needs
+        :param int plan_sdram: minimum amount of SDRAM needed
+        :rtype: Chip or None
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
         """
-        # If we are reporting a used chip, update with reachable chips
-        if last_chip_space is not None:
-            last_chip = last_chip_space.chip
-            self.__add_neighbours(last_chip)
-
-        # If no space, error
-        if not self.__has_neighbour():
-            self.__last_chip_space = None
-            logger.debug("No more chips to place on in this space; "
-                         "{} of {} used",
-                         self.n_chips_used, self.__machine.n_chips)
-            return None
-        chip = self.__pop_neighbour()
-        self.__used_chips.add(chip)
-        if chip in self.__restored_chips:
-            self.__restored_chips.remove(chip)
-        cores_used, sdram_used = self.__cores_and_sdram(chip)
-        self.__last_chip_space = _ChipWithSpace(chip, cores_used, sdram_used)
-        return self.__last_chip_space
-
-    @property
-    def n_chips_used(self) -> int:
+        # Do while Chip with space not found
+        while True:
+            chip = self.__pop_neighbour()
+            if chip is None:
+                # Sign to consider preparation with this start a failure
+                return None
+            if self.__space_on_chip(chip, n_cores, plan_sdram):
+                return chip
+
+    def get_next_chip_with_space(
+            self, n_cores: int, plan_sdram: int) -> Optional[Chip]:
         """
-        The number of chips used.
+        Gets the next Chip with space
 
-        :rtype: int
+        If no start Chip is available raise an Exception
+        If no neighbouring more Chips available returns None
+
+        :param int n_cores: number of cores needs
+        :param int plan_sdram: minimum amount of SDRAM needed
+        :raises PacmanPlaceException: If no new start Chip is available
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
         """
-        return len(self.__used_chips)
+        if self.__current_chip is None:
+            return self.__get_next_start(n_cores, plan_sdram)
+        elif (len(self.__current_cores_free) >= n_cores and
+              self.__current_sdram_free >= plan_sdram):
+            # Cores are popped out later
+            self.__current_sdram_free -= plan_sdram
+            return self.__current_chip
+        else:
+            return self.__get_next_neighbour(n_cores, plan_sdram)
 
     def __add_neighbours(self, chip: Chip):
         for link in chip.router.links:
             target = self.__machine[link.destination_x, link.destination_y]
-            if target not in self.__used_chips:
+            if (target not in self.__full_chips
+                    and target not in self.__prepared_chips):
                 if (target.nearest_ethernet_x == self.__ethernet_x and
                         target.nearest_ethernet_y == self.__ethernet_y):
                     self.__same_board_chips[target] = target
                 else:
                     self.__other_board_chips[target] = target
 
-    def __has_neighbour(self):
-        return (len(self.__same_board_chips) > 0 or
-                len(self.__other_board_chips) > 0)
-
-    def __pop_neighbour(self):
+    def __pop_neighbour(self) -> Optional[Chip]:
         if self.__same_board_chips:
             k = next(iter(self.__same_board_chips))
             del self.__same_board_chips[k]
@@ -532,103 +701,14 @@ def __pop_neighbour(self):
                 else:
                     self.__other_board_chips[chip] = chip
             return next_chip
-        raise StopIteration
 
-    def save_chip(self, chip: Chip):
-        """
-        Marks a Chip as to be used by the current Vertex
-        :param Chip chip:
-        """
-        self.__saved_chips.append(chip)
+        # Signal that there are no more Chips with a None
+        return None
 
-    def chips_saved(self) -> List[Chip]:
-        """
-        The chips saved but not yet permanently used
-
-        :rtype:list(Chip)
-        """
-        return self.__saved_chips
-
-    def restore_chips(self) -> None:
-        """
-        Moves all the saved chips form used to restored
-        """
-        for chip in self.__saved_chips:
-            self.__used_chips.remove(chip)
-            self.__restored_chips.append(chip)
-        self.__saved_chips.clear()
-
-    def clear_saved(self) -> None:
-        """
-        Clears the saved Chip which makes the permanently used
+    def pop_next_core(self):
         """
-        self.__saved_chips.clear()
-
-
-class _ChipWithSpace(object):
-    """
-    A chip with space for placement.
-    """
-    __slots__ = ("chip", "cores", "sdram")
-
-    def __init__(
-            self, chip: Chip, used_processors: Set[int], used_sdram: int):
-        """
-
-        :param Chip chip:
-        :param set(int) used_processors:
-        :param int used_sdram:
-        """
-        self.chip = chip
-        self.cores = set(chip.placable_processors_ids)
-        self.cores -= used_processors
-        self.sdram = chip.sdram - used_sdram
-
-    @property
-    def x(self) -> int:
-        """
-        The x value of the Chip passed in at init time
-
-        :rtype: int
-        """
-        return self.chip.x
-
-    @property
-    def y(self) -> int:
-        """
-        The y value of the Chip passed in at init time
-
-        :rtype: int
-        """
-        return self.chip.y
-
-    def is_space(self, n_cores: int, sdram: int) -> bool:
-        """
-        CHecks if there is space based on both cores and sdram
-
-        :param int n_cores:
-        :param int sdram:
-        :rtype: bool
-        """
-        return len(self.cores) >= n_cores and self.sdram >= sdram
-
-    def use_sdram(self, sdram: int):
-        """
-        Reduces available sdram by this amount
-
-        :param int sdram:
-        """
-        self.sdram -= sdram
-
-    def use_next_core(self) -> int:
-        """
-        Pops a core value
+        Get the next free core on the current Chip
 
         :rtype: int
         """
-        core = next(iter(self.cores))
-        self.cores.remove(core)
-        return core
-
-    def __repr__(self):
-        return f"({self.x}, {self.y})"
+        return self.__current_cores_free.pop(0)
\ No newline at end of file
diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index 857b3ebb9..082817e17 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -19,7 +19,7 @@
 from pacman.model.partitioner_splitters import (
     SplitterFixedLegacy, AbstractSplitterCommon)
 from pacman.operations.placer_algorithms.application_placer import (
-    place_application_graph, _check_could_fit)
+    place_application_graph, _Spaces)
 from pacman.model.graphs.machine import SimpleMachineVertex
 from pacman.model.resources import ConstantSDRAM
 from pacman.model.graphs.application import ApplicationVertex
@@ -141,7 +141,7 @@ def test_application_placer_too_few_boards():
         place_application_graph(Placements())
         raise AssertionError("Error not raise")
     except PacmanPlaceException as ex:
-        assert ("No more chips to place" in str(ex))
+        assert ("No more chips to start" in str(ex))
 
 
 def test_application_placer_restart_needed():
@@ -201,7 +201,8 @@ def test_sdram_bigger_monitors():
     # This is purely an info call so test check directly
     writer.add_sample_monitor_vertex(monitor, True)
     try:
-        _check_could_fit("app_test", ["m_vertex]"], sdram=max_sdram // 2 + 5)
+        spaces = _Spaces(Placements())
+        spaces._Spaces__check_could_fit(1, plan_sdram=max_sdram // 2 + 5)
         raise AssertionError("Error not raise")
     except PacmanTooBigToPlace as ex:
         assert ("after monitors only" in str(ex))
@@ -238,9 +239,9 @@ def test_more_cores_with_monitor():
     monitor = SimpleMachineVertex(ConstantSDRAM(4000))
     # This is purely an info call so test check directly
     writer.add_sample_monitor_vertex(monitor, True)
-    m_vertexs = [f"m_v_{i}" for i in range(17)]
     try:
-        _check_could_fit("app_test", m_vertexs, 500000)
+        spaces = _Spaces(Placements())
+        spaces._Spaces__check_could_fit(17, 500000)
         raise AssertionError("Error not raise")
     except PacmanTooBigToPlace as ex:
         assert ("reserved for monitors" in str(ex))
@@ -252,5 +253,5 @@ def test_could_fit():
     writer = PacmanDataWriter.mock()
     monitor = SimpleMachineVertex(ConstantSDRAM(0))
     writer.add_sample_monitor_vertex(monitor, True)
-    m_vertexs = [f"m_v_{i}" for i in range(16)]
-    _check_could_fit("app_test", m_vertexs, 500000)
+    spaces = _Spaces(Placements())
+    spaces._Spaces__check_could_fit(16, 500000)

From 69ce85c3c16c41236290ae3772618cf9b3e700dd Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 14:11:52 +0000
Subject: [PATCH 26/33] change numbers coming out of placer

---
 .../placer_algorithms_tests/test_application_placer.py    | 2 +-
 .../routing_table_generator_tests/test_basic.py           | 6 +++---
 .../routing_table_generator_tests/test_merged.py          | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index 082817e17..6aac91c8b 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -102,7 +102,7 @@ def test_application_placer():
     fixed.set_fixed_location(0, 0)
     writer.add_vertex(fixed)
     fixed.splitter.create_machine_vertices(ChipCounter())
-    for i in range(56):
+    for i in range(61):
         _make_vertices(writer, 1000, 14, 5, f"app_vertex_{i}")
     writer.set_machine(virtual_machine(24, 12))
     place_application_graph(Placements())
diff --git a/unittests/operations_tests/routing_table_generator_tests/test_basic.py b/unittests/operations_tests/routing_table_generator_tests/test_basic.py
index 9456f1718..90a6d3536 100644
--- a/unittests/operations_tests/routing_table_generator_tests/test_basic.py
+++ b/unittests/operations_tests/routing_table_generator_tests/test_basic.py
@@ -81,6 +81,6 @@ def test_graph3_with_system(self):
         system_plaements.add_placement(Placement(mv, 1, 2, 3))
         self.make_infos(writer, system_plaements)
         data = basic_routing_table_generator()
-        self.assertEqual(34, data.get_max_number_of_entries())
-        self.assertEqual(114, data.get_total_number_of_entries())
-        self.assertEqual(4, len(list(data.routing_tables)))
+        self.assertEqual(31, data.get_max_number_of_entries())
+        self.assertEqual(108, data.get_total_number_of_entries())
+        self.assertEqual(5, len(list(data.routing_tables)))
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 8bd5d6cac..d57ac6821 100644
--- a/unittests/operations_tests/routing_table_generator_tests/test_merged.py
+++ b/unittests/operations_tests/routing_table_generator_tests/test_merged.py
@@ -114,8 +114,8 @@ def test_graph2(self):
         self.create_graphs3(writer)
         self.make_infos(writer)
         data = merged_routing_table_generator()
-        self.assertEqual(10, data.get_max_number_of_entries())
-        self.assertEqual(4, len(list(data.routing_tables)))
+        self.assertEqual(7, data.get_max_number_of_entries())
+        self.assertEqual(5, len(list(data.routing_tables)))
 
     def test_graph3_with_system(self):
         writer = PacmanDataWriter.mock()
@@ -125,8 +125,8 @@ def test_graph3_with_system(self):
         system_plaements.add_placement(Placement(mv, 1, 2, 3))
         self.make_infos(writer, system_plaements)
         data = merged_routing_table_generator()
-        self.assertEqual(10, data.get_max_number_of_entries())
-        self.assertEqual(4, len(list(data.routing_tables)))
+        self.assertEqual(7, data.get_max_number_of_entries())
+        self.assertEqual(5, len(list(data.routing_tables)))
 
     def test_bad_infos(self):
         writer = PacmanDataWriter.mock()

From f1cbaedc1b2d7db098445a32ca7d47e4ba3c9ad2 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 14:30:52 +0000
Subject: [PATCH 27/33] Add a test for filling in chips.

---
 .../test_application_placer.py                 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index 6aac91c8b..ac40bd05c 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -178,6 +178,24 @@ def test_application_placer_late_fixed():
     place_application_graph(Placements())
 
 
+def test_application_placer_fill_chips():
+    unittest_setup()
+    set_config("Machine", "version", 5)
+    writer = PacmanDataWriter.mock()
+    # fixed early works as this vertex is looked at first
+    fixed = SimpleTestVertex(10, "FIXED", max_atoms_per_core=1)
+    fixed.splitter = SplitterFixedLegacy()
+    fixed.set_fixed_location(0, 0)
+    writer.add_vertex(fixed)
+    fixed.splitter.create_machine_vertices(ChipCounter())
+    for i in range(17):
+        _make_vertices(writer, 1000, 14, 9, f"app_vertex_{i}")
+    for i in range(17):
+        _make_vertices(writer, 1000, 14, 8, f"app_vertex_{i}")
+    writer.set_machine(virtual_machine(24, 12))
+    place_application_graph(Placements())
+
+
 def test_sdram_bigger_than_chip():
     unittest_setup()
     set_config("Machine", "version", 5)

From 30be5093a8ee6264eea3d8b088ebb1cc3e36df29 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 14:48:24 +0000
Subject: [PATCH 28/33] flake8

---
 .../placer_algorithms/application_placer.py   | 65 ++-----------------
 1 file changed, 7 insertions(+), 58 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 9fbd16da1..a9ddd439d 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -14,7 +14,7 @@
 from __future__ import annotations
 import logging
 import os
-from typing import Dict, List, Optional, Set, Tuple
+from typing import Dict, List, Optional, Set
 
 from spinn_utilities.config_holder import get_config_bool
 from spinn_utilities.log import FormatAdapter
@@ -90,7 +90,7 @@ def _place_vertex(
             spaces, placements, plan_n_timesteps, same_chip_groups)
         if placements_to_make is not None:
             break
-    
+
     # Now actually add the placements having confirmed all can be done
     placements.add_placements(placements_to_make)
 
@@ -226,46 +226,6 @@ def _place_error(
         f" Report written to {report_file}.")
 
 
-def _check_could_fit(
-        app_vertex: ApplicationVertex, vertices_to_place: List[MachineVertex],
-        sdram: int):
-    """
-    :param ApplicationVertex app_vertex:
-    :param list(MachineVertex) vertices_to_place:
-    :param int sdram:
-    :raises PacmanTooBigToPlace:
-    """
-    n_cores = len(vertices_to_place)
-    if sdram <= self.__max_sdram and n_cores <= self.__max_cores:
-        # should fit somewhere
-        return
-    message = (
-        f"{app_vertex} will not fit on any possible Chip "
-        f"the reason is that {vertices_to_place} ")
-    if sdram > max_sdram:
-        message += f"requires {sdram} bytes but "
-        if sdram > version.max_sdram_per_chip:
-            message += f"a Chip only has {version.max_sdram_per_chip} bytes "
-        else:
-            message += f"after monitors only {max_sdram} bytes are available "
-        message += "Lowering max_core_per_chip may resolve this."
-        raise PacmanTooBigToPlace(message)
-    if n_cores > version.max_cores_per_chip:
-        message += " is more vertices than the number of cores on a chip."
-        raise PacmanTooBigToPlace(message)
-    user_cores = version.max_cores_per_chip - version.n_scamp_cores
-    if n_cores > user_cores:
-        message += (
-            f"is more vertices than the user cores ({user_cores}) "
-            "available on a Chip")
-    else:
-        message += (
-            f"is more vertices than the {max_cores} cores available on a "
-            f"Chip once {PacmanDataView.get_all_monitor_cores()} "
-            "are reserved for monitors")
-    raise PacmanTooBigToPlace(message)
-
-
 def _place_fixed_vertex(
         app_vertex: ApplicationVertex, placements: Placements):
     same_chip_groups = app_vertex.splitter.get_same_chip_groups()
@@ -326,17 +286,6 @@ def _do_fixed_location(
                     f"No more cores available on {x}, {y}: {on_chip}")
 
 
-def _store_on_chip(
-        placements_to_make: List[Placement], vertices: List[MachineVertex],
-        sdram: int, next_chip_space: _ChipWithSpace):
-    """
-    :param list(Placement) placements_to_make:
-    :param list(MachineVertex) vertices:
-    :param int sdram:
-    :param _ChipWithSpace next_chip_space:
-    """
-
-
 class _Spaces(object):
     __slots__ = (
         # Values from PacmanDataView cached for speed
@@ -525,7 +474,7 @@ def __space_on_chip(
 
         return True
 
-    def __check_could_fit(self, n_cores:int, plan_sdram: int):
+    def __check_could_fit(self, n_cores: int, plan_sdram: int):
         """
         :param int n_cores: number of cores needs
         :param int plan_sdram: minimum amount of SDRAM needed
@@ -570,10 +519,10 @@ def __check_could_fit(self, n_cores:int, plan_sdram: int):
     def __get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
         """
         Gets the next start Chip
-        
+
         Also sets up the current_chip and starts a new neighbourhood
-         
-        :param int n_cores: number of cores needs 
+
+        :param int n_cores: number of cores needs
         :param int plan_sdram: minimum amount of SDRAM needed
 
         :rtype: Chip
@@ -711,4 +660,4 @@ def pop_next_core(self):
 
         :rtype: int
         """
-        return self.__current_cores_free.pop(0)
\ No newline at end of file
+        return self.__current_cores_free.pop(0)

From 54d8e913af7049c4c287e31eafc2bae3c752a86c Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 15:38:01 +0000
Subject: [PATCH 29/33] create an ApplicationPlacer class

---
 .../placer_algorithms/application_placer.py   | 532 +++++++++---------
 1 file changed, 252 insertions(+), 280 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index a9ddd439d..54b35c396 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -45,248 +45,11 @@ def place_application_graph(system_placements: Placements) -> Placements:
     :return: Placements for the application. *Includes the system placements.*
     :rtype: Placements
     """
-    # Track the placements and space
-    placements = Placements(system_placements)
-
-    # Go through the application graph by application vertex
-    progress = ProgressBar(
-        PacmanDataView.get_n_vertices() * 2, "Placing Vertices")
-    try:
-        for app_vertex in progress.over(
-                PacmanDataView.iterate_vertices(), finish_at_end=False):
-            if app_vertex.has_fixed_location():
-                _place_fixed_vertex(app_vertex, placements)
-
-        plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
-        spaces = _Spaces(placements)
-        for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
-            # as this checks if placed already not need to check if fixed
-            _place_vertex(app_vertex, spaces, plan_n_timesteps, placements)
-    except PacmanPlaceException as e:
-        raise _place_error(
-            placements, system_placements, e,
-            plan_n_timesteps) from e
-
-    if get_config_bool("Reports", "draw_placements"):
-        # pylint: disable=import-outside-toplevel
-        from .draw_placements import draw_placements as dp
-        dp(placements, system_placements)
-
-    return placements
-
-
-def _place_vertex(
-        app_vertex: ApplicationVertex, spaces: _Spaces,
-        plan_n_timesteps: Optional[int], placements: Placements):
-    same_chip_groups = app_vertex.splitter.get_same_chip_groups()
-    if not same_chip_groups:
-        # This vertex does not require placement or delegates
-        return
-
-    spaces.start_app_vertex(app_vertex)
-    # try to make placements with a different start Chip each time
-    while True:
-        placements_to_make = _prepare_placements(
-            spaces, placements, plan_n_timesteps, same_chip_groups)
-        if placements_to_make is not None:
-            break
-
-    # Now actually add the placements having confirmed all can be done
-    placements.add_placements(placements_to_make)
-
-
-def _prepare_placements(
-        spaces, placements, plan_n_timesteps, same_chip_groups):
-    spaces.start_preparing()
-    placements_to_make: List = list()
-
-    # Go through the groups
-    for vertices, sdram in same_chip_groups:
-        vertices_to_place = _filter_vertices(vertices, placements)
-        if len(vertices_to_place) == 0:
-            continue
-        plan_sdram = sdram.get_total_sdram(plan_n_timesteps)
-        n_cores = len(vertices_to_place)
-
-        # Try to find a chip with space
-        chip = spaces.get_next_chip_with_space(n_cores, plan_sdram)
-        if chip is None:
-            return None
-
-        # If this worked, store placements to be made
-        for vertex in vertices:
-            core = spaces.pop_next_core()
-            placements_to_make.append(Placement(
-                vertex, chip.x, chip.y, core))
-    return placements_to_make
-
-
-def _filter_vertices(vertices, placements):
-    # Remove any already placed
-    vertices_to_place = [
-        vertex for vertex in vertices
-        if not placements.is_vertex_placed(vertex)]
-    if len(vertices_to_place) != len(vertices) and len(vertices_to_place) > 0:
-        # Putting the rest on a different chip is wrong
-        # Putting them on the same chip is hard so will do if needed
-        raise NotImplementedError(
-            "Unexpected mix of placed and unplaced vertices")
-    # No need to place virtual vertices
-    return [vertex for vertex in vertices_to_place
-            if not isinstance(vertex, AbstractVirtual)]
-
-
-def _place_error(
-        placements: Placements, system_placements: Placements,
-        exception: Exception,
-        plan_n_timesteps: Optional[int]) -> PacmanPlaceException:
-    """
-    :param Placements placements:
-    :param Placements system_placements:
-    :param PacmanPlaceException exception:
-    :param int plan_n_timesteps:
-    :rtype: PacmanPlaceException
-    """
-    unplaceable = list()
-    vertex_count = 0
-    n_vertices = 0
-    for app_vertex in PacmanDataView.iterate_vertices():
-        same_chip_groups = app_vertex.splitter.get_same_chip_groups()
-        app_vertex_placed = True
-        found_placed_cores = False
-        for vertices, _sdram in same_chip_groups:
-            if isinstance(vertices[0], AbstractVirtual):
-                break
-            if placements.is_vertex_placed(vertices[0]):
-                found_placed_cores = True
-            elif found_placed_cores:
-                vertex_count += len(vertices)
-                n_vertices = len(same_chip_groups)
-                app_vertex_placed = False
-                break
-            else:
-                app_vertex_placed = False
-                break
-        if not app_vertex_placed:
-            unplaceable.append(app_vertex)
-
-    report_file = os.path.join(
-        PacmanDataView.get_run_dir_path(), "placements_error.txt")
-    with open(report_file, 'w', encoding="utf-8") as f:
-        f.write(f"Could not place {len(unplaceable)} of "
-                f"{PacmanDataView.get_n_vertices()} application vertices.\n")
-        f.write(f"    Could not place {vertex_count} of {n_vertices} in the"
-                " last app vertex\n\n")
-        for xy in placements.chips_with_placements:
-            first = True
-            for placement in placements.placements_on_chip(xy):
-                if system_placements.is_vertex_placed(placement.vertex):
-                    continue
-                if first:
-                    f.write(f"Chip ({xy}):\n")
-                    first = False
-                f.write(f"    Processor {placement.p}:"
-                        f" Vertex {placement.vertex}\n")
-            if not first:
-                f.write("\n")
-        f.write("\n")
-        f.write("Not placed:\n")
-        for app_vertex in unplaceable:
-            f.write(f"Vertex: {app_vertex}\n")
-            same_chip_groups = app_vertex.splitter.get_same_chip_groups()
-            for vertices, sdram in same_chip_groups:
-                f.write(f"    Group of {len(vertices)} vertices uses "
-                        f"{sdram.get_total_sdram(plan_n_timesteps)} "
-                        "bytes of SDRAM:\n")
-                for vertex in vertices:
-                    f.write(f"        Vertex {vertex}")
-                    if placements.is_vertex_placed(vertex):
-                        plce = placements.get_placement_of_vertex(vertex)
-                        f.write(f" (placed at {plce.x}, {plce.y}, {plce.p})")
-                    f.write("\n")
-
-        f.write("\n")
-        f.write("Unused chips:\n")
-        machine = PacmanDataView.get_machine()
-        for xy in machine.chip_coordinates:
-            n_placed = placements.n_placements_on_chip(xy)
-            system_placed = system_placements.n_placements_on_chip(xy)
-            if n_placed - system_placed == 0:
-                n_procs = machine[xy].n_placable_processors
-                f.write(f"    {xy} ({n_procs - system_placed}"
-                        " free cores)\n")
-
-    if get_config_bool("Reports", "draw_placements_on_error"):
-        # pylint: disable=import-outside-toplevel
-        from .draw_placements import draw_placements as dp
-        dp(placements, system_placements)
+    placer = ApplicationPlacer(system_placements)
+    return placer._do_placements(system_placements)
 
-    return PacmanPlaceException(
-        f" {exception}."
-        f" Report written to {report_file}.")
 
-
-def _place_fixed_vertex(
-        app_vertex: ApplicationVertex, placements: Placements):
-    same_chip_groups = app_vertex.splitter.get_same_chip_groups()
-    if not same_chip_groups:
-        raise NotImplementedError("Unexpected mix of Fixed and no groups")
-
-    for vertices, _ in same_chip_groups:
-        vertices_to_place = _filter_vertices(vertices, placements)
-        _do_fixed_location(vertices_to_place, placements)
-
-
-def _do_fixed_location(
-        vertices: list[MachineVertex], placements: Placements):
-    """
-    :param list(MachineVertex) vertices:
-    :param Placements placements:
-    :rtype: bool
-    :raise PacmanConfigurationException:
-    """
-    for vertex in vertices:
-        loc = vertex.get_fixed_location()
-        if loc:
-            x, y = loc.x, loc.y
-            break
-    else:
-        # Mixing fixed and free allocations while still keeping the whole
-        # App vertex together is hard so will only do is needed
-        raise NotImplementedError(
-            "Mixing fixed location and not fixed location groups "
-            "within one vertex")
-
-    chip = PacmanDataView.get_chip_at(x, y)
-    if chip is None:
-        raise PacmanConfigurationException(
-            f"Constrained to chip {x, y} but no such chip")
-    on_chip = placements.placements_on_chip(chip)
-    cores_used = {p.p for p in on_chip}
-    cores = set(chip.placable_processors_ids) - cores_used
-    next_cores = iter(cores)
-    # first do the ones with a fixed p
-    for vertex in vertices:
-        fixed = vertex.get_fixed_location()
-        if fixed and fixed.p is not None:
-            if fixed.p not in next_cores:
-                raise PacmanConfigurationException(
-                    f"Core {fixed.p} on {x}, {y} not available to "
-                    f"place {vertex} on")
-            placements.add_placement(Placement(vertex, x, y, fixed.p))
-    # Then do the ones without a fixed p
-    for vertex in vertices:
-        fixed = vertex.get_fixed_location()
-        if not fixed or fixed.p is None:
-            try:
-                placements.add_placement(Placement(vertex, x, y, next(next_cores)))
-            except StopIteration:
-                # pylint: disable=raise-missing-from
-                raise PacmanConfigurationException(
-                    f"No more cores available on {x}, {y}: {on_chip}")
-
-
-class _Spaces(object):
+class ApplicationPlacer(object):
     __slots__ = (
         # Values from PacmanDataView cached for speed
         # PacmanDataView.get_machine()
@@ -332,11 +95,6 @@ class _Spaces(object):
         # List of available neighbours not on the current board
         "__other_board_chips")
 
-    # __last_chip_space
-    # __used_chip
-    # __saved_chip
-    # __nextChip, next_start
-
     def __init__(self, placements: Placements):
         """
         :param Placements placements:
@@ -356,7 +114,7 @@ def __init__(self, placements: Placements):
         self.__min_sdram = self.__max_sdram // self.__max_cores
 
         self.__placements = placements
-        self.__chips = self.__chip_order()
+        self.__chips = self._chip_order()
 
         self.__full_chips: Set[Chip] = set()
         self.__prepared_chips: Set[Chip] = set()
@@ -374,29 +132,251 @@ def __init__(self, placements: Placements):
         self.__same_board_chips: Dict[Chip, Chip] = dict()
         self.__other_board_chips:  Dict[Chip, Chip] = dict()
 
-    def start_app_vertex(self, app_vertex: ApplicationVertex):
+    def _do_placements(self, system_placements: Placements) -> Placements:
         """
-        Signal that the next Application vertex is starting
-        :param ApplicationVertex app_vertex:
+        Perform placement of an application graph on the machine.
+
+        .. note::
+            app_graph must have been partitioned
+
+        :param Placements system_placements:
+            The placements of cores doing system tasks.
+        :return: Placements for the application.
+            *Includes the system placements.*
+        :rtype: Placements
         """
-        # Store the label for error reporting
+        # Go through the application graph by application vertex
+        progress = ProgressBar(
+            PacmanDataView.get_n_vertices() * 2, "Placing Vertices")
+        try:
+            for app_vertex in progress.over(
+                    PacmanDataView.iterate_vertices(), finish_at_end=False):
+                if app_vertex.has_fixed_location():
+                    self._place_fixed_vertex(app_vertex)
+
+            plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
+            for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
+                # as this checks if placed already not need to check if fixed
+                self._place_vertex(app_vertex)
+        except PacmanPlaceException as e:
+            raise self._place_error(system_placements, e) from e
+
+        if get_config_bool("Reports", "draw_placements"):
+            # pylint: disable=import-outside-toplevel
+            from .draw_placements import draw_placements as dp
+            dp(self.__placements, system_placements)
+
+        return self.__placements
+
+    def _place_vertex(self, app_vertex: ApplicationVertex,):
+        same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+        if not same_chip_groups:
+            # This vertex does not require placement or delegates
+            return
+
         self.__app_vertex = app_vertex.label
 
         # Restore the starts tried last time.
         # Check if they are full comes later
         while len(self.__starts_tried):
             self.__restored_chips.append(self.__starts_tried.pop(0))
-        self.start_preparing()
 
-    def start_preparing(self) -> None:
-        """
-        Signal that a new attempt to prepared placements is starting
-        """
+        # try to make placements with a different start Chip each time
+        while True:
+            placements_to_make = self._prepare_placements(same_chip_groups)
+            if placements_to_make is not None:
+                break
+
+        # Now actually add the placements having confirmed all can be done
+        self.__placements.add_placements(placements_to_make)
+
+    def _prepare_placements(self, same_chip_groups):
         # Clear the Chips used in the last prepare
         self.__prepared_chips.clear()
         self.__current_chip = None
 
-    def __chip_order(self):
+        placements_to_make: List = list()
+
+        # Go through the groups
+        for vertices, sdram in same_chip_groups:
+            vertices_to_place = self._filter_vertices(vertices)
+            if len(vertices_to_place) == 0:
+                continue
+            plan_sdram = sdram.get_total_sdram(self.__plan_n_timesteps)
+            n_cores = len(vertices_to_place)
+
+            # Try to find a chip with space
+            chip = self._get_next_chip_with_space(n_cores, plan_sdram)
+            if chip is None:
+                return None
+
+            # If this worked, store placements to be made
+            for vertex in vertices:
+                core = self.__current_cores_free.pop(0)
+                placements_to_make.append(Placement(
+                    vertex, chip.x, chip.y, core))
+        return placements_to_make
+
+    def _filter_vertices(self, vertices):
+        # Remove any already placed
+        vertices_to_place = [
+            vertex for vertex in vertices
+            if not self.__placements.is_vertex_placed(vertex)]
+        if len(vertices_to_place) != len(vertices) and len(
+                vertices_to_place) > 0:
+            # Putting the rest on a different chip is wrong
+            # Putting them on the same chip is hard so will do if needed
+            raise NotImplementedError(
+                "Unexpected mix of placed and unplaced vertices")
+        # No need to place virtual vertices
+        return [vertex for vertex in vertices_to_place
+                if not isinstance(vertex, AbstractVirtual)]
+
+    def _place_error(self, system_placements: Placements,
+                     exception: Exception) -> PacmanPlaceException:
+        """
+        :param Placements system_placements:
+        :param PacmanPlaceException exception:
+        :rtype: PacmanPlaceException
+        """
+        unplaceable = list()
+        vertex_count = 0
+        n_vertices = 0
+        for app_vertex in PacmanDataView.iterate_vertices():
+            same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+            app_vertex_placed = True
+            found_placed_cores = False
+            for vertices, _sdram in same_chip_groups:
+                if isinstance(vertices[0], AbstractVirtual):
+                    break
+                if self.__placements.is_vertex_placed(vertices[0]):
+                    found_placed_cores = True
+                elif found_placed_cores:
+                    vertex_count += len(vertices)
+                    n_vertices = len(same_chip_groups)
+                    app_vertex_placed = False
+                    break
+                else:
+                    app_vertex_placed = False
+                    break
+            if not app_vertex_placed:
+                unplaceable.append(app_vertex)
+
+        report_file = os.path.join(
+            PacmanDataView.get_run_dir_path(), "placements_error.txt")
+        with open(report_file, 'w', encoding="utf-8") as f:
+            f.write(f"Could not place {len(unplaceable)} of "
+                    f"{PacmanDataView.get_n_vertices()} application vertices.\n")
+            f.write(
+                f"    Could not place {vertex_count} of {n_vertices} in the"
+                " last app vertex\n\n")
+            for xy in self.__placements.chips_with_placements:
+                first = True
+                for placement in self.__placements.placements_on_chip(xy):
+                    if system_placements.is_vertex_placed(placement.vertex):
+                        continue
+                    if first:
+                        f.write(f"Chip ({xy}):\n")
+                        first = False
+                    f.write(f"    Processor {placement.p}:"
+                            f" Vertex {placement.vertex}\n")
+                if not first:
+                    f.write("\n")
+            f.write("\n")
+            f.write("Not placed:\n")
+            for app_vertex in unplaceable:
+                f.write(f"Vertex: {app_vertex}\n")
+                same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+                for vertices, sdram in same_chip_groups:
+                    f.write(f"    Group of {len(vertices)} vertices uses "
+                            f"{sdram.get_total_sdram(self.__plan_n_timesteps)} "
+                            "bytes of SDRAM:\n")
+                    for vertex in vertices:
+                        f.write(f"        Vertex {vertex}")
+                        if self.__placements.is_vertex_placed(vertex):
+                            plce = self.__placements.get_placement_of_vertex(
+                                vertex)
+                            f.write(
+                                f" (placed at {plce.x}, {plce.y}, {plce.p})")
+                        f.write("\n")
+
+            f.write("\n")
+            f.write("Unused chips:\n")
+            machine = PacmanDataView.get_machine()
+            for xy in machine.chip_coordinates:
+                n_placed = self.__placements.n_placements_on_chip(xy)
+                system_placed = system_placements.n_placements_on_chip(xy)
+                if n_placed - system_placed == 0:
+                    n_procs = machine[xy].n_placable_processors
+                    f.write(f"    {xy} ({n_procs - system_placed}"
+                            " free cores)\n")
+
+        if get_config_bool("Reports", "draw_placements_on_error"):
+            # pylint: disable=import-outside-toplevel
+            from .draw_placements import draw_placements as dp
+            dp(self.__placements, system_placements)
+
+        return PacmanPlaceException(
+            f" {exception}."
+            f" Report written to {report_file}.")
+
+    def _place_fixed_vertex(self, app_vertex: ApplicationVertex):
+        same_chip_groups = app_vertex.splitter.get_same_chip_groups()
+        if not same_chip_groups:
+            raise NotImplementedError("Unexpected mix of Fixed and no groups")
+
+        for vertices, _ in same_chip_groups:
+            vertices_to_place = self._filter_vertices(vertices)
+            self._do_fixed_location(vertices_to_place)
+
+    def _do_fixed_location(self, vertices: list[MachineVertex]):
+        """
+        :param list(MachineVertex) vertices:
+        :raise PacmanConfigurationException:
+        """
+        for vertex in vertices:
+            loc = vertex.get_fixed_location()
+            if loc:
+                x, y = loc.x, loc.y
+                break
+        else:
+            # Mixing fixed and free allocations while still keeping the whole
+            # App vertex together is hard so will only do is needed
+            raise NotImplementedError(
+                "Mixing fixed location and not fixed location groups "
+                "within one vertex")
+
+        chip = PacmanDataView.get_chip_at(x, y)
+        if chip is None:
+            raise PacmanConfigurationException(
+                f"Constrained to chip {x, y} but no such chip")
+        on_chip = self.__placements.placements_on_chip(chip)
+        cores_used = {p.p for p in on_chip}
+        cores = set(chip.placable_processors_ids) - cores_used
+        next_cores = iter(cores)
+        # first do the ones with a fixed p
+        for vertex in vertices:
+            fixed = vertex.get_fixed_location()
+            if fixed and fixed.p is not None:
+                if fixed.p not in next_cores:
+                    raise PacmanConfigurationException(
+                        f"Core {fixed.p} on {x}, {y} not available to "
+                        f"place {vertex} on")
+                self.__placements.add_placement(
+                    Placement(vertex, x, y, fixed.p))
+        # Then do the ones without a fixed p
+        for vertex in vertices:
+            fixed = vertex.get_fixed_location()
+            if not fixed or fixed.p is None:
+                try:
+                    self.__placements.add_placement(
+                        Placement(vertex, x, y, next(next_cores)))
+                except StopIteration:
+                    # pylint: disable=raise-missing-from
+                    raise PacmanConfigurationException(
+                        f"No more cores available on {x}, {y}: {on_chip}")
+
+    def _chip_order(self):
         """
         Iterate the Chips in a guaranteed order
 
@@ -409,7 +389,7 @@ def __chip_order(self):
                 if chip:
                     yield chip
 
-    def __space_on_chip(
+    def _space_on_chip(
             self, chip: Chip, n_cores: int, plan_sdram: int) -> bool:
         """
         Checks if the Chip has enough space for this group, Cache if yes
@@ -459,7 +439,7 @@ def __space_on_chip(
         self.__prepared_chips.add(chip)
 
         if len(cores_free) < n_cores or sdram_free < plan_sdram:
-            self.__check_could_fit(n_cores, plan_sdram)
+            self._check_could_fit(n_cores, plan_sdram)
             return False
 
         # record the current Chip
@@ -470,11 +450,11 @@ def __space_on_chip(
         self.__current_sdram_free = sdram_free - plan_sdram
 
         # adds the neighburs
-        self.__add_neighbours(chip)
+        self._add_neighbours(chip)
 
         return True
 
-    def __check_could_fit(self, n_cores: int, plan_sdram: int):
+    def _check_could_fit(self, n_cores: int, plan_sdram: int):
         """
         :param int n_cores: number of cores needs
         :param int plan_sdram: minimum amount of SDRAM needed
@@ -516,7 +496,7 @@ def __check_could_fit(self, n_cores: int, plan_sdram: int):
                 f"are reserved for monitors")
         raise PacmanTooBigToPlace(message)
 
-    def __get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
+    def _get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
         """
         Gets the next start Chip
 
@@ -536,19 +516,19 @@ def __get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
 
         # Find the next start chip
         while True:
-            start = self.__pop_start_chip()
+            start = self._pop_start_chip()
             # Save as tried as not full even if toO small
             self.__starts_tried.append(start)
             # Set the ethernets in case space_on_chip adds neighbours
             self.__ethernet_x = start.nearest_ethernet_x
             self.__ethernet_y = start.nearest_ethernet_y
-            if self.__space_on_chip(start, n_cores, plan_sdram):
+            if self._space_on_chip(start, n_cores, plan_sdram):
                 break
 
         logger.debug("Starting placement from {}", start)
         return start
 
-    def __pop_start_chip(self) -> Chip:
+    def _pop_start_chip(self) -> Chip:
         """
         Gets the next start Chip from either restored or if none the Machine
 
@@ -573,7 +553,7 @@ def __pop_start_chip(self) -> Chip:
                 f"{len(self.__full_chips)} already full "
                 f"and {len(self.__starts_tried)} tried")
 
-    def __get_next_neighbour(self, n_cores: int, plan_sdram: int):
+    def _get_next_neighbour(self, n_cores: int, plan_sdram: int):
         """
         Gets the next neighbour Chip
 
@@ -589,14 +569,14 @@ def __get_next_neighbour(self, n_cores: int, plan_sdram: int):
         """
         # Do while Chip with space not found
         while True:
-            chip = self.__pop_neighbour()
+            chip = self._pop_neighbour()
             if chip is None:
                 # Sign to consider preparation with this start a failure
                 return None
-            if self.__space_on_chip(chip, n_cores, plan_sdram):
+            if self._space_on_chip(chip, n_cores, plan_sdram):
                 return chip
 
-    def get_next_chip_with_space(
+    def _get_next_chip_with_space(
             self, n_cores: int, plan_sdram: int) -> Optional[Chip]:
         """
         Gets the next Chip with space
@@ -611,16 +591,16 @@ def get_next_chip_with_space(
             If the requirements are too big for any chip
         """
         if self.__current_chip is None:
-            return self.__get_next_start(n_cores, plan_sdram)
+            return self._get_next_start(n_cores, plan_sdram)
         elif (len(self.__current_cores_free) >= n_cores and
               self.__current_sdram_free >= plan_sdram):
             # Cores are popped out later
             self.__current_sdram_free -= plan_sdram
             return self.__current_chip
         else:
-            return self.__get_next_neighbour(n_cores, plan_sdram)
+            return self._get_next_neighbour(n_cores, plan_sdram)
 
-    def __add_neighbours(self, chip: Chip):
+    def _add_neighbours(self, chip: Chip):
         for link in chip.router.links:
             target = self.__machine[link.destination_x, link.destination_y]
             if (target not in self.__full_chips
@@ -631,7 +611,7 @@ def __add_neighbours(self, chip: Chip):
                 else:
                     self.__other_board_chips[target] = target
 
-    def __pop_neighbour(self) -> Optional[Chip]:
+    def _pop_neighbour(self) -> Optional[Chip]:
         if self.__same_board_chips:
             k = next(iter(self.__same_board_chips))
             del self.__same_board_chips[k]
@@ -653,11 +633,3 @@ def __pop_neighbour(self) -> Optional[Chip]:
 
         # Signal that there are no more Chips with a None
         return None
-
-    def pop_next_core(self):
-        """
-        Get the next free core on the current Chip
-
-        :rtype: int
-        """
-        return self.__current_cores_free.pop(0)

From 0584754497e3a1b605aa8b14b0551d51ea1c0f8a Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Fri, 22 Mar 2024 16:36:05 +0000
Subject: [PATCH 30/33] comments and minor fixes

---
 .../placer_algorithms/application_placer.py   | 113 ++++++++++++++----
 .../test_application_placer.py                |  17 ++-
 2 files changed, 100 insertions(+), 30 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 54b35c396..169e6d255 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -14,7 +14,7 @@
 from __future__ import annotations
 import logging
 import os
-from typing import Dict, List, Optional, Set
+from typing import Dict, List, Optional, Tuple, Sequence, Set
 
 from spinn_utilities.config_holder import get_config_bool
 from spinn_utilities.log import FormatAdapter
@@ -27,6 +27,7 @@
 from pacman.model.graphs import AbstractVirtual
 from pacman.model.graphs.machine import MachineVertex
 from pacman.model.graphs.application import ApplicationVertex
+from pacman.model.resources import AbstractSDRAM
 from pacman.exceptions import (
     PacmanPlaceException, PacmanConfigurationException, PacmanTooBigToPlace)
 
@@ -46,10 +47,14 @@ def place_application_graph(system_placements: Placements) -> Placements:
     :rtype: Placements
     """
     placer = ApplicationPlacer(system_placements)
-    return placer._do_placements(system_placements)
+    return placer.do_placements(system_placements)
 
 
 class ApplicationPlacer(object):
+    """
+    Places the Vertices keeping ones for an ApplicationVertex together.
+
+    """
     __slots__ = (
         # Values from PacmanDataView cached for speed
         # PacmanDataView.get_machine()
@@ -58,14 +63,14 @@ class ApplicationPlacer(object):
         "__plan_n_timesteps",
         # Sdram available on perfect none Ethernet Chip after Monitors placed
         "__max_sdram",
-        # Minumum sdram that should be available for a Chip to not be full
+        # Minimum sdram that should be available for a Chip to not be full
         "__min_sdram",
         # N Cores free on perfect none Ethernet Chip after Monitors placed
         "__max_cores",
 
         # Pointer to the placements including all previous Application Vertices
         "__placements",
-        # A Function to yield the Chips in a consistant order
+        # A Function to yield the Chips in a consistent order
         "__chips",
         # Chips that have been fully placed by previous Application Vertices
         "__full_chips",
@@ -76,7 +81,7 @@ class ApplicationPlacer(object):
         # Start Chips tried for this ApplicationVertex
         "__starts_tried",
         # Label of the current ApplicationVertex for (error) reporting
-        "__app_vertex",
+        "__app_vertex_label",
 
         # Data for the last Chip offered to place on
         # May be full after current group placed
@@ -124,7 +129,7 @@ def __init__(self, placements: Placements):
         self.__current_chip: Optional[Chip] = None
         self.__current_cores_free: List[int] = list()
         self.__current_sdram_free = 0
-        self.__app_vertex = "NO APP VETERX SET"
+        self.__app_vertex_label = "NO APP VETERX SET"
 
         # Set some value so no Optional needed
         self.__ethernet_x = -1
@@ -132,7 +137,7 @@ def __init__(self, placements: Placements):
         self.__same_board_chips: Dict[Chip, Chip] = dict()
         self.__other_board_chips:  Dict[Chip, Chip] = dict()
 
-    def _do_placements(self, system_placements: Placements) -> Placements:
+    def do_placements(self, system_placements: Placements) -> Placements:
         """
         Perform placement of an application graph on the machine.
 
@@ -144,6 +149,9 @@ def _do_placements(self, system_placements: Placements) -> Placements:
         :return: Placements for the application.
             *Includes the system placements.*
         :rtype: Placements
+        :raises PacmanPlaceException: If no new start Chip is available
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
         """
         # Go through the application graph by application vertex
         progress = ProgressBar(
@@ -154,7 +162,6 @@ def _do_placements(self, system_placements: Placements) -> Placements:
                 if app_vertex.has_fixed_location():
                     self._place_fixed_vertex(app_vertex)
 
-            plan_n_timesteps = PacmanDataView.get_plan_n_timestep()
             for app_vertex in progress.over(PacmanDataView.iterate_vertices()):
                 # as this checks if placed already not need to check if fixed
                 self._place_vertex(app_vertex)
@@ -168,17 +175,25 @@ def _do_placements(self, system_placements: Placements) -> Placements:
 
         return self.__placements
 
-    def _place_vertex(self, app_vertex: ApplicationVertex,):
+    def _place_vertex(self, app_vertex: ApplicationVertex):
+        """
+        Place the next application vertex
+
+        :param ApplicationVertex app_vertex:
+        :raises PacmanPlaceException: If no new start Chip is available
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
+        """
         same_chip_groups = app_vertex.splitter.get_same_chip_groups()
         if not same_chip_groups:
             # This vertex does not require placement or delegates
             return
 
-        self.__app_vertex = app_vertex.label
+        self.__app_vertex_label = app_vertex.label
 
         # Restore the starts tried last time.
         # Check if they are full comes later
-        while len(self.__starts_tried):
+        while len(self.__starts_tried) > 0:
             self.__restored_chips.append(self.__starts_tried.pop(0))
 
         # try to make placements with a different start Chip each time
@@ -190,7 +205,28 @@ def _place_vertex(self, app_vertex: ApplicationVertex,):
         # Now actually add the placements having confirmed all can be done
         self.__placements.add_placements(placements_to_make)
 
-    def _prepare_placements(self, same_chip_groups):
+    def _prepare_placements(self, same_chip_groups:  Sequence[
+            Tuple[Sequence[MachineVertex], AbstractSDRAM]]
+            ) -> Optional[List[Placement]]:
+        """
+        Try to make the placements for this ApplicationVertex.
+
+        This will make sure all placements are on linked Chips
+
+        The next start Chip is tried.
+
+        If successful a list of created but NOT added placements is returned
+
+        If this start Chip fails it returns None.
+        A start chip could fail either because it does not have enough space
+        or because its neighbours do not have enough space
+
+
+        :param list(list(MachineVertex), AbstractSdram) same_chip_groups:
+        :raises PacmanPlaceException: If no new start Chip is available
+        :raises PacmanTooBigToPlace:
+            If the requirements are too big for any chip
+        """
         # Clear the Chips used in the last prepare
         self.__prepared_chips.clear()
         self.__current_chip = None
@@ -201,6 +237,7 @@ def _prepare_placements(self, same_chip_groups):
         for vertices, sdram in same_chip_groups:
             vertices_to_place = self._filter_vertices(vertices)
             if len(vertices_to_place) == 0:
+                # Either placed (fixed) or virtual so skip group
                 continue
             plan_sdram = sdram.get_total_sdram(self.__plan_n_timesteps)
             n_cores = len(vertices_to_place)
@@ -217,7 +254,16 @@ def _prepare_placements(self, same_chip_groups):
                     vertex, chip.x, chip.y, core))
         return placements_to_make
 
-    def _filter_vertices(self, vertices):
+    def _filter_vertices(
+            self, vertices: List[MachineVertex]) -> List[MachineVertex]:
+        """
+        Removes an already placed or virtual vertices.
+
+        Errors on groups that have both placed and unplaced vertices!
+
+        :param vertices:
+        :rtype: List(MachineVertex)
+        """
         # Remove any already placed
         vertices_to_place = [
             vertex for vertex in vertices
@@ -321,6 +367,13 @@ def _place_error(self, system_placements: Placements,
             f" Report written to {report_file}.")
 
     def _place_fixed_vertex(self, app_vertex: ApplicationVertex):
+        """
+        Place all vertices for this Application Vertex
+
+        Checks that all MachineVertices are fixed or errors
+
+        :param ApplicationVertex app_vertex:
+        """
         same_chip_groups = app_vertex.splitter.get_same_chip_groups()
         if not same_chip_groups:
             raise NotImplementedError("Unexpected mix of Fixed and no groups")
@@ -331,8 +384,13 @@ def _place_fixed_vertex(self, app_vertex: ApplicationVertex):
 
     def _do_fixed_location(self, vertices: list[MachineVertex]):
         """
+        Do fixed placing for one group.
+
+        Errors if the group does not have a fixed location
+
         :param list(MachineVertex) vertices:
         :raise PacmanConfigurationException:
+            If the requested location is not available
         """
         for vertex in vertices:
             loc = vertex.get_fixed_location()
@@ -380,7 +438,6 @@ def _chip_order(self):
         """
         Iterate the Chips in a guaranteed order
 
-        :param Machine machine:
         :rtype: iterable(Chip)
         """
         for x in range(self.__machine.width):
@@ -407,12 +464,12 @@ def _space_on_chip(
         The values Cached are the:
         current_chip Even if full to keep the code simpler
         current_cores_free Including the ones for this group
-        current_sdram_free Excluding the sdram needed fot this group
+        current_sdram_free Excluding the sdram needed fit this group
 
         :param Chip chip:
         :param int n_cores: number of cores needed
         :param int plan_sdram:
-        :rtype: tuple(int, int)
+        :rtype: bool
         :raises PacmanTooBigToPlace:
             If the requirements are too big for any chip
         """
@@ -449,13 +506,15 @@ def _space_on_chip(
         # sdram is the whole group so can be removed now
         self.__current_sdram_free = sdram_free - plan_sdram
 
-        # adds the neighburs
+        # adds the neighbours
         self._add_neighbours(chip)
 
         return True
 
     def _check_could_fit(self, n_cores: int, plan_sdram: int):
         """
+        Checks that the cores/SDRAM would fit on a empty perfect Chip
+
         :param int n_cores: number of cores needs
         :param int plan_sdram: minimum amount of SDRAM needed
         :raises PacmanTooBigToPlace:
@@ -465,7 +524,7 @@ def _check_could_fit(self, n_cores: int, plan_sdram: int):
             # should fit somewhere
             return
         message = (
-            f"{self.__app_vertex} will not fit on any possible Chip "
+            f"{self.__app_vertex_label} will not fit on any possible Chip "
             f"as a smae_chip_group ")
 
         version = PacmanDataView.get_machine_version()
@@ -519,7 +578,7 @@ def _get_next_start(self, n_cores: int, plan_sdram: int) -> Chip:
             start = self._pop_start_chip()
             # Save as tried as not full even if toO small
             self.__starts_tried.append(start)
-            # Set the ethernets in case space_on_chip adds neighbours
+            # Set the Ethernet x and y in case space_on_chip adds neighbours
             self.__ethernet_x = start.nearest_ethernet_x
             self.__ethernet_y = start.nearest_ethernet_y
             if self._space_on_chip(start, n_cores, plan_sdram):
@@ -548,7 +607,7 @@ def _pop_start_chip(self) -> Chip:
             return start
         except StopIteration:
             raise PacmanPlaceException(  # pylint: disable=raise-missing-from
-                f"No more chips to start with for {self.__app_vertex} "
+                f"No more chips to start with for {self.__app_vertex_label} "
                 f"Out of {self.__machine.n_chips} "
                 f"{len(self.__full_chips)} already full "
                 f"and {len(self.__starts_tried)} tried")
@@ -559,7 +618,7 @@ def _get_next_neighbour(self, n_cores: int, plan_sdram: int):
 
         Also changes the current_chip and updates the neighbourhood
 
-        This wil return None if there are no more neighbouring Chip big enough
+        This will return None if there are no more neighbouring Chip big enough
 
         :param int n_cores: number of cores needs
         :param int plan_sdram: minimum amount of SDRAM needed
@@ -601,6 +660,12 @@ def _get_next_chip_with_space(
             return self._get_next_neighbour(n_cores, plan_sdram)
 
     def _add_neighbours(self, chip: Chip):
+        """
+        Adds the neighbours for this Chip to be used as the next chips.
+
+        :param Chip chip:
+        :return:
+        """
         for link in chip.router.links:
             target = self.__machine[link.destination_x, link.destination_y]
             if (target not in self.__full_chips
@@ -612,6 +677,12 @@ def _add_neighbours(self, chip: Chip):
                     self.__other_board_chips[target] = target
 
     def _pop_neighbour(self) -> Optional[Chip]:
+        """
+        Pops the next neighbour Chip with preference to ones on current board
+
+        :return: A neighbour Chip or None if there are no More
+        :rtype: None
+        """
         if self.__same_board_chips:
             k = next(iter(self.__same_board_chips))
             del self.__same_board_chips[k]
diff --git a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
index ac40bd05c..2da335dce 100644
--- a/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
+++ b/unittests/operations_tests/placer_algorithms_tests/test_application_placer.py
@@ -14,12 +14,11 @@
 from spinn_utilities.config_holder import set_config
 from spinn_machine.virtual_machine import virtual_machine
 from pacman.data.pacman_data_writer import PacmanDataWriter
-from pacman.exceptions import (
-    PacmanConfigurationException, PacmanPlaceException, PacmanTooBigToPlace)
+from pacman.exceptions import (PacmanPlaceException, PacmanTooBigToPlace)
 from pacman.model.partitioner_splitters import (
     SplitterFixedLegacy, AbstractSplitterCommon)
 from pacman.operations.placer_algorithms.application_placer import (
-    place_application_graph, _Spaces)
+    place_application_graph, ApplicationPlacer)
 from pacman.model.graphs.machine import SimpleMachineVertex
 from pacman.model.resources import ConstantSDRAM
 from pacman.model.graphs.application import ApplicationVertex
@@ -219,8 +218,8 @@ def test_sdram_bigger_monitors():
     # This is purely an info call so test check directly
     writer.add_sample_monitor_vertex(monitor, True)
     try:
-        spaces = _Spaces(Placements())
-        spaces._Spaces__check_could_fit(1, plan_sdram=max_sdram // 2 + 5)
+        placer = ApplicationPlacer(Placements())
+        placer._check_could_fit(1, plan_sdram=max_sdram // 2 + 5)
         raise AssertionError("Error not raise")
     except PacmanTooBigToPlace as ex:
         assert ("after monitors only" in str(ex))
@@ -258,8 +257,8 @@ def test_more_cores_with_monitor():
     # This is purely an info call so test check directly
     writer.add_sample_monitor_vertex(monitor, True)
     try:
-        spaces = _Spaces(Placements())
-        spaces._Spaces__check_could_fit(17, 500000)
+        placer = ApplicationPlacer(Placements())
+        placer._check_could_fit(17, 500000)
         raise AssertionError("Error not raise")
     except PacmanTooBigToPlace as ex:
         assert ("reserved for monitors" in str(ex))
@@ -271,5 +270,5 @@ def test_could_fit():
     writer = PacmanDataWriter.mock()
     monitor = SimpleMachineVertex(ConstantSDRAM(0))
     writer.add_sample_monitor_vertex(monitor, True)
-    spaces = _Spaces(Placements())
-    spaces._Spaces__check_could_fit(16, 500000)
+    placer = ApplicationPlacer(Placements())
+    placer._check_could_fit(16, 500000)

From 726f0324772622ff33242d4274171d183b54721a Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 25 Mar 2024 07:15:54 +0000
Subject: [PATCH 31/33] flake8

---
 pacman/operations/placer_algorithms/application_placer.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index 169e6d255..cc2e5ef35 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -312,7 +312,8 @@ def _place_error(self, system_placements: Placements,
             PacmanDataView.get_run_dir_path(), "placements_error.txt")
         with open(report_file, 'w', encoding="utf-8") as f:
             f.write(f"Could not place {len(unplaceable)} of "
-                    f"{PacmanDataView.get_n_vertices()} application vertices.\n")
+                    f"{PacmanDataView.get_n_vertices()} "
+                    f"application vertices.\n")
             f.write(
                 f"    Could not place {vertex_count} of {n_vertices} in the"
                 " last app vertex\n\n")
@@ -334,9 +335,9 @@ def _place_error(self, system_placements: Placements,
                 f.write(f"Vertex: {app_vertex}\n")
                 same_chip_groups = app_vertex.splitter.get_same_chip_groups()
                 for vertices, sdram in same_chip_groups:
+                    sdram = sdram.get_total_sdram(self.__plan_n_timesteps)
                     f.write(f"    Group of {len(vertices)} vertices uses "
-                            f"{sdram.get_total_sdram(self.__plan_n_timesteps)} "
-                            "bytes of SDRAM:\n")
+                            f"{sdram} bytes of SDRAM:\n")
                     for vertex in vertices:
                         f.write(f"        Vertex {vertex}")
                         if self.__placements.is_vertex_placed(vertex):

From 40ba0fd9d57943e3f51fd4371b80861ffe77005f Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 25 Mar 2024 07:42:20 +0000
Subject: [PATCH 32/33] typing fixes

---
 pacman/model/placements/placements.py                     | 4 ++--
 pacman/operations/placer_algorithms/application_placer.py | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/pacman/model/placements/placements.py b/pacman/model/placements/placements.py
index 684740001..b90cd6e0f 100644
--- a/pacman/model/placements/placements.py
+++ b/pacman/model/placements/placements.py
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 from collections import defaultdict
-from typing import Dict, Iterable, Iterator
+from typing import Collection, Dict, Iterable, Iterator
 
 from spinn_utilities.typing.coords import XY
 
@@ -197,7 +197,7 @@ def placements(self) -> Iterable[Placement]:
         """
         return iter(self._machine_vertices.values())
 
-    def placements_on_chip(self, xy: XY) -> Iterable[Placement]:
+    def placements_on_chip(self, xy: XY) -> Collection[Placement]:
         """
         Get the placements on a specific chip.
 
diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index cc2e5ef35..d53aebc81 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -129,7 +129,7 @@ def __init__(self, placements: Placements):
         self.__current_chip: Optional[Chip] = None
         self.__current_cores_free: List[int] = list()
         self.__current_sdram_free = 0
-        self.__app_vertex_label = "NO APP VETERX SET"
+        self.__app_vertex_label = None
 
         # Set some value so no Optional needed
         self.__ethernet_x = -1
@@ -255,7 +255,7 @@ def _prepare_placements(self, same_chip_groups:  Sequence[
         return placements_to_make
 
     def _filter_vertices(
-            self, vertices: List[MachineVertex]) -> List[MachineVertex]:
+            self, vertices: Sequence[MachineVertex]) -> List[MachineVertex]:
         """
         Removes an already placed or virtual vertices.
 
@@ -335,9 +335,9 @@ def _place_error(self, system_placements: Placements,
                 f.write(f"Vertex: {app_vertex}\n")
                 same_chip_groups = app_vertex.splitter.get_same_chip_groups()
                 for vertices, sdram in same_chip_groups:
-                    sdram = sdram.get_total_sdram(self.__plan_n_timesteps)
+                    p_sdram = sdram.get_total_sdram(self.__plan_n_timesteps)
                     f.write(f"    Group of {len(vertices)} vertices uses "
-                            f"{sdram} bytes of SDRAM:\n")
+                            f"{p_sdram} bytes of SDRAM:\n")
                     for vertex in vertices:
                         f.write(f"        Vertex {vertex}")
                         if self.__placements.is_vertex_placed(vertex):

From 23ed9eb64e9f2f2acbe1fa73c525e1d68c00c978 Mon Sep 17 00:00:00 2001
From: "Christian Y. Brenninkmeijer"
 <christian.brenninkmeijer@manchester.ac.uk>
Date: Mon, 25 Mar 2024 07:47:06 +0000
Subject: [PATCH 33/33] more typing

---
 pacman/operations/placer_algorithms/application_placer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pacman/operations/placer_algorithms/application_placer.py b/pacman/operations/placer_algorithms/application_placer.py
index d53aebc81..dc01ab153 100644
--- a/pacman/operations/placer_algorithms/application_placer.py
+++ b/pacman/operations/placer_algorithms/application_placer.py
@@ -129,7 +129,7 @@ def __init__(self, placements: Placements):
         self.__current_chip: Optional[Chip] = None
         self.__current_cores_free: List[int] = list()
         self.__current_sdram_free = 0
-        self.__app_vertex_label = None
+        self.__app_vertex_label: Optional[str] = None
 
         # Set some value so no Optional needed
         self.__ethernet_x = -1