Skip to content

Commit a140ca6

Browse files
committed
Address additional code review feedback
- Change normalize guard from `< EPSILON` to `<= 0.0` - only return zero vector for actual zero magnitude, not near-zero values - Use `val.abs` with EPSILON threshold for singular values to handle small negative values from floating point errors more robustly - Update test to reflect correct behavior: near-zero vectors normalize to unit vectors, only true zero vectors return zero
1 parent 806503a commit a140ca6

File tree

2 files changed

+11
-7
lines changed

2 files changed

+11
-7
lines changed

lib/classifier/extensions/vector.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ def magnitude
2929

3030
def normalize
3131
magnitude_value = magnitude
32-
# Return zero vector if magnitude is too small to safely divide
33-
return Vector[*Array.new(size, 0.0)] if magnitude_value < EPSILON
32+
# Return zero vector only if magnitude is zero or numerically negative
33+
return Vector[*Array.new(size, 0.0)] if magnitude_value <= 0.0
3434

3535
normalized_values = []
3636
size.times do |i|
@@ -102,7 +102,7 @@ def SV_decomp(max_sweeps = 20)
102102
q_rotation_matrix.row_size.times do |r|
103103
# Guard against negative values due to floating point errors
104104
val = q_rotation_matrix[r, r].to_f
105-
singular_values << Math.sqrt(val.negative? ? 0.0 : val)
105+
singular_values << Math.sqrt(val < -Vector::EPSILON ? 0.0 : val.abs)
106106
end
107107

108108
# Replace near-zero singular values with EPSILON to prevent division by zero

test/extensions/word_hash_test.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,17 @@ def test_normalize_zero_vector_returns_zero_vector
125125
assert_in_delta 0.0, normalized[2], 0.001
126126
end
127127

128-
def test_normalize_near_zero_vector_returns_zero_vector
128+
def test_normalize_near_zero_vector_returns_unit_vector
129+
# Near-zero vectors should still normalize to unit vectors
130+
# Only actual zero vectors return zero
129131
vec = Vector[1e-15, 1e-15, 1e-15]
130132
normalized = vec.normalize
131133

132-
assert_in_delta 0.0, normalized[0], 0.001
133-
assert_in_delta 0.0, normalized[1], 0.001
134-
assert_in_delta 0.0, normalized[2], 0.001
134+
# Should normalize to [1/sqrt(3), 1/sqrt(3), 1/sqrt(3)]
135+
expected = 1.0 / Math.sqrt(3)
136+
assert_in_delta expected, normalized[0], 0.001
137+
assert_in_delta expected, normalized[1], 0.001
138+
assert_in_delta expected, normalized[2], 0.001
135139
end
136140
end
137141

0 commit comments

Comments
 (0)