Skip to content

fix bug in normalize #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
suborofu wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix bug in normalize #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions stepfg/_geometry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ def normalize(vector_in):
if len(vector_in) != 3:
raise ValueError("normalize: coordinates must be 3D")
magnitude = math.sqrt(sum(i ** 2 for i in vector_in))
if magnitude == 0: # in some cases vector_in has zeros only
return [0 for _ in vector_in]
Comment on lines +14 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find all callers of normalize() to verify they handle zero vector returns

# Find all calls to normalize()
rg -n -B3 -A3 'normalize\(' --type=py -g '!test_*.py' -g '!*_test.py'

Repository: evvaletov/stepfg

Length of output: 2850

Fix zero-vector handling for normalize() results used as STEP directions

  • stepfg/_builder.py passes normalize(...) results directly into STEP DIRECTION and _axis2_placement_3d inputs (e.g., normalize(direction); normalize(zaxis) / normalize(vertices[*] - vertices[0]); and normalize(cross_product(...))), so a [0, 0, 0] return would propagate into emitted geometry.
  • The check if list(map(operator.add, normalize(zaxis), normalize(cp))) == [0, 0, 0]: only flips winding; it does not prevent emitting [0, 0, 0] direction vectors.

Handle zero magnitude either by making normalize() raise (or otherwise refuse), or by adding a well-defined fallback at these call sites, and add a test covering zero-vector normalization.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@stepfg/_geometry.py` around lines 14 - 15, The normalize() routine currently
returns [0,0,0] for zero magnitude which propagates into STEP
DIRECTION/_axis2_placement_3d; change behavior to either (a) make
normalize(vector) raise a clear ValueError on zero magnitude (update function
normalize in stepfg._geometry) or (b) keep normalize but add explicit fallbacks
at callers in stepfg._builder where normalize(direction), normalize(zaxis),
normalize(vertices[i] - vertices[0]) and normalize(cross_product(...)) are used:
detect zero magnitude and substitute a well-defined default unit axis (e.g.,
[1,0,0] or compute a non-parallel axis) or recompute a valid orthonormal, and
add a unit test exercising normalization of a zero vector to ensure the resolver
path is exercised; pick one approach and apply consistently across the
referenced call sites and tests.

return [x / magnitude for x in vector_in]


Expand Down