Skip to content

Y-sorting-threshold comparator is not transitive, causes node-order inversions #214

Description

@webdevred

Problem

compareY in compareAV (src-extra/transformation/JbeamEdit/Transformation.hs:342-361) uses a pairwise "fuzzy equality" comparator:

compareY =
  let yDiff = abs $ y1 - y2
   in bool EQ (compare y1 y2) (yDiff > thr)

Two nodes are treated as the same Y-band if their distance is <= thr. This is not transitive: AB and BC does not imply A~C, and sortBy assumes transitivity. When a chain of nodes has consecutive gaps under thr but a total spread over thr, the resulting order depends on merge-sort comparison order rather than actual Y position.

TRANSFORMATION-TODO.md already documents this failure mode for bolide_chassis (the f1l/f5l/f6l cluster) and calibrated the default y-sorting-threshold to 0.05 specifically to avoid it across the existing 238-file survey. That calibration is fragile: it depends on the specific node spacing of whatever files were tested, not a property of the algorithm.

Repro

Using gen4_body (a NASCAR-style body file, not currently in the example corpus) with y-sorting-threshold: 0.1, support-threshold: 20, running --transform produces order inversions exceeding the threshold at both ends of the vehicle:

  • nll47 (Y=-1.967, the frontmost node in its group) sorts to the last position in the group, after nll1 (Y=-1.807), a gap of 0.16 which is greater than the 0.1 threshold.
  • nrr41 (Y=2.258) sorts before nrr43 (Y=2.139), also a gap greater than the threshold.

Confirmed on both master (d6bc439) and triangles-excluded-from-support-vertices, same result, this code path is unrelated to and unaffected by either branch's recent changes.

Structural integrity (beams/triangles by node position) is unaffected, this is purely a node-ordering/index-assignment defect, not a dropped connection.

Suggested direction

Replace the pairwise fuzzy comparator with a precomputed band assignment:

  1. Sort the group by raw Y ascending.
  2. Walk the sorted list once, assigning an incrementing integer band index whenever the gap to the previous element exceeds thr.
  3. Sort by (band, Z, X) instead of (fuzzy-Y, Z, X).

An Int band index is transitive by construction, so this removes the whole bug class regardless of what y-sorting-threshold value is configured, no more per-file calibration needed.

Test gap

None of the existing fixtures (fender, frame, suspension) reproduce this, and the shared examples/jbeam-edit.yaml test config uses y-sorting-threshold: 0.05 (the calibrated-safe value), so the existing suite would not catch a regression here. Needs either a small fixture with known problematic Y-spacing, or a targeted unit test on the banding function directly.

Fix branch: fix-y-sorting-transitivity

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions