Make Line3 relation tests invariant to Plücker scale - #232
Doribelove wants to merge 2 commits into
Conversation
Sorry, something went wrong.
Normalize line directions and moments for parallel and intersection checks. Construct common perpendiculars from the nearest point and unit direction cross product, with regression coverage across Plucker scalings. Assisted-by: OpenAI Codex (GPT-6 Sol) Signed-off-by: 李永祺 <doribelove@gmail.com>
|
Thank you for the quick follow-up, and for closing out #231 so thoroughly! I independently verified this rather than just re-running your own tests: random-sampled ~10,000 scaled line pairs each for isparallel() and isintersecting() (scale factors from 1e-8 to 1e8, including sign flips) and confirmed the classification never changes with scale. For commonperp() I checked it geometrically rather than via distance() (since this branch doesn't include #224's fix) — for ~2,600 random scaled skew-line pairs, the returned perpendicular line passes through the correct nearest point, is perpendicular to both inputs, and lands exactly on the second line when stepped by the true separation distance. Zero failures across all of that. Also confirmed this merges cleanly against #224 with no conflicts, and both sets of tests pass together. Approving — nice work, thanks again. |
Sorry, something went wrong.
Keep the near-parallel tolerance test focused on direction vectors by constructing both lines through the origin. This avoids an unrelated Plucker orthogonality check failure on NumPy 2. Assisted-by: OpenAI Codex (GPT-6 Sol) Signed-off-by: 李永祺 <doribelove@gmail.com>
|
Thank you for independently validating the geometry and approving the PR. I found one test-fixture failure in the previous head's codecov job: the large, offset near-parallel line was rejected by PointDir's Plücker orthogonality check due to floating-point cancellation before isparallel() ran. Follow-up 149653a changes only that fixture to pass through the origin; its direction and the angular-tolerance assertion are unchanged. On the new head, the full suite passes locally with Python 3.10.12 / NumPy 2.2.6 (350 passed, 3 skipped) and Python 3.12.14 / NumPy 2.5.3 under pytest-cov (350 passed, 3 skipped). Black and git diff --check also pass. The new fork-PR CI run is currently action_required, so I am not claiming a remote result for this commit yet. The previous Sphinx failure is the known fork publishing issue tracked separately by #222. This follow-up was prepared and validated with OpenAI Codex assistance on behalf of @Doribelove. |
Sorry, something went wrong.
Summary
Line3.isparallel() compared the raw cross product of Plücker direction vectors to an absolute tolerance. Scaling either representation therefore changed whether perpendicular lines were reported as parallel. The same classification feeds |, isintersecting() / ^, and commonperp().
For example, lines through (0, 0, 0) along (1e-8, 0, 0) and through (0, 0, 1) along (0, 1e-8, 0) are perpendicular; before this change isparallel() returned True.
This branch is based on current master and does not contain the separate distance() fix in #224. I checked closest_to_line() on the included scaled nonparallel cases; it needs no change for this issue.
Validation
Fixes #231.
Implementation and tests were prepared and validated with OpenAI Codex assistance.