From f3fd79e8ff96087ee1b6474acee5b8fe8f8897f8 Mon Sep 17 00:00:00 2001 From: "Vincent T.L." Date: Thu, 7 May 2026 21:49:04 -0400 Subject: [PATCH] Fix off-by-one in __get_grid_cell_location at canvas extents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: The private helper __get_grid_cell_location returns out-of-range (row, col) when an input coordinate sits exactly at the canvas extent. Most internal call sites do not clamp the result, so the next routing-array indexing raises IndexError and crashes plc.get_congestion_cost() on otherwise-legal placements. Reproduction (with any minimal benchmark loaded): plc.set_canvas_size(W, H) plc.set_placement_grid(grid_col=10, grid_row=10) row, col = plc._PlacementCost__get_grid_cell_location(W, H) assert (row, col) == (9, 9) # FAILS pre-fix: returns (10, 10) Root Cause: row = math.floor(y_pos / self.grid_height) col = math.floor(x_pos / self.grid_width) When y_pos == height (or x_pos == width) exactly, math.floor(height / (height/grid_row)) == grid_row, which is one past the last legal index `grid_row - 1`. The routing-congestion arrays are sized grid_row * grid_col (lines 117-118, 1150-1151) and indexed as `arr[row * grid_col + col]` (lines 1291, 1297, 1310, 1315). So `row == grid_row` or `col == grid_col` produces a flat-index `>= len(arr)` and raises IndexError. Solution: Clamp `row` and `col` into [0, grid_row-1] / [0, grid_col-1] before returning. This is bit-exact with the prior implementation for all in-canvas inputs (y < height AND x < width); the clamps are no-ops in that range. Only at-canvas-extent inputs change behavior — from "out-of-range index, crash downstream" to "last in-range index, no crash" — which is the obviously-intended semantics given the docstring's claim that the result ranges "from 0...N". Why this matters: Of 13 call sites for __get_grid_cell_location in plc_client_os.py, 9 do not clamp the result on the caller side (lines 1409, 1544, 1554, 1559, 1560, 1572, 1883, 2102, 2178). Only compute_routing (line 1008) has explicit caller-side clamps at lines 1023-1027. Fixing the helper itself covers all callers in one place. Analytical placers commonly clamp macro centers to [half_w, canvas_width - half_w], which puts the macro EDGE at exactly canvas_width — the trigger condition. Without this fix, those placers cannot evaluate congestion on legal placements. Testing: - Bit-exact for interior coordinates: get_grid_cell_location(50, 50) returns (5, 5) on a 100x100 canvas with 10x10 grid, both pre and post fix. - At canvas extent: get_grid_cell_location(100, 100) returns (9, 9) post fix; pre-fix it returned (10, 10). - Verified on the IBM benchmark suite: get_congestion_cost() now succeeds at all canvas-edge macro positions; previously crashed with IndexError. Impact: This fix unblocks any analytical or gradient-based placer that clamps macro centers to the natural [half_w, canvas - half_w] range. Combined with the existing scientific-notation fix (45a721d), the upstream plc_client_os is now usable on the full ICCAD04 IBM benchmark suite without contestant-side defensive patches. Signed-off-by: Vincent Trudel-Lapierre --- CodeElements/Plc_client/plc_client_os.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CodeElements/Plc_client/plc_client_os.py b/CodeElements/Plc_client/plc_client_os.py index 773e77e..a14cce5 100644 --- a/CodeElements/Plc_client/plc_client_os.py +++ b/CodeElements/Plc_client/plc_client_os.py @@ -919,6 +919,10 @@ def __get_grid_cell_location(self, x_pos, y_pos): self.grid_height = float(self.height/self.grid_row) row = math.floor(y_pos / self.grid_height) col = math.floor(x_pos / self.grid_width) + # Clamp to legal range [0, grid_*-1] — floor() returns grid_* + # at the canvas extent, one past the last valid routing index. + row = max(0, min(row, self.grid_row - 1)) + col = max(0, min(col, self.grid_col - 1)) return row, col def __get_grid_location_position(self, col:int, row:int):