Skip to content

Comments

Fix Precision Bugs in ConvexHull and SaccadeVelocity#70

Merged
oscararenas12 merged 1 commit intomainfrom
bug-fixes-oa
Dec 6, 2025
Merged

Fix Precision Bugs in ConvexHull and SaccadeVelocity#70
oscararenas12 merged 1 commit intomainfrom
bug-fixes-oa

Conversation

@oscararenas12
Copy link
Contributor

Summary

  • Fix floating-point truncation in ConvexHull angle/distance calculations
  • Fix incorrect 0.0 return value in SaccadeVelocity when all velocities exceed threshold
  • Fix test file line endings for OpenCSV 5.x compatibility

Changes

1. ConvexHull.java - Floating-Point Truncation Fix

Problem: The (long) casts truncated decimal portions of coordinates before performing calculations, causing precision loss in angle and distance computations.

Affected locations:

  • Line 183-184: Angle calculation (thetaA, thetaB)
  • Line 195-198: Distance calculation (distanceA, distanceB)
  • Line 236-237: Cross product in getTurn()

Example of precision loss:

Coordinates: a.x = 800.789, a.y = 523.456, lowest.x = 200.123, lowest.y = 100.789

Java (before - with truncation):
  Y: (long)523.456 - 100.789 = 523 - 100.789 = 422.211
  X: (long)800.789 - 200.123 = 800 - 200.123 = 599.877
  theta = atan2(422.211, 599.877) = 0.6135 radians

Python/Fixed (full precision):
  Y: 523.456 - 100.789 = 422.667
  X: 800.789 - 200.123 = 600.666
  theta = atan2(422.667, 600.666) = 0.6138 radians

Before:

// use longs to guard against int-underflow
double thetaA = Math.atan2((long)a.y - lowest.y, (long)a.x - lowest.x);
double thetaB = Math.atan2((long)b.y - lowest.y, (long)b.x - lowest.x);

After:

double thetaA = Math.atan2(a.y - lowest.y, a.x - lowest.x);
double thetaB = Math.atan2(b.y - lowest.y, b.x - lowest.x);

Why the original comment was wrong: The code uses Point2D.Double which has double fields. There's no integer overflow/underflow risk - the (long) casts were unnecessary and harmful.


2. SaccadeVelocity.java - Zero Velocity Bug Fix

Problem: When all velocities in a saccade exceeded the 700°/s threshold, getPeakVelocity() returned 0.0 instead of NaN. This 0.0 was included in aggregate statistics, artificially lowering min/mean/median values.

Before:

double peakVelocity = 0;

for (int i = 1; i < saccadePoints.size(); i++) {
    // ... calculate velocity ...
    if (velocity > peakVelocity && velocity <= VELOCITY_THRESHOLD) {
        peakVelocity = velocity;
    }
}

return peakVelocity;  // Returns 0.0 if nothing passed threshold

After:

double peakVelocity = 0;
boolean foundValidVelocity = false;

for (int i = 1; i < saccadePoints.size(); i++) {
    // ... calculate velocity ...
    if (velocity <= VELOCITY_THRESHOLD) {
        if (velocity > peakVelocity) {
            peakVelocity = velocity;
        }
        foundValidVelocity = true;
    }
}

return foundValidVelocity ? peakVelocity : Double.NaN;

Impact on statistics:

Condition Before After
All velocities > 700 Returns 0.0 (included in stats) Returns NaN (excluded)
Some velocities <= 700 Returns peak Returns peak
< 2 data points Returns NaN Returns NaN

Example:

Java list before (51 values, includes 0.0):
[118.0, 245.3, ..., 0.0, ..., 312.8]
min = 0.0, mean = 176.34

Java list after (50 values, excludes invalid):
[118.0, 245.3, ..., 312.8]
min = 2.87, mean = 176.65

3. SaccadeVelocityTest.java - Updated Test

Updated test to expect NaN instead of 0 when all velocities exceed threshold:

Before:

@Test
public void testGetPeakVelocity_aboveThreshold_return0() {
    // ...
    boolean isZero = SaccadeVelocity.getPeakVelocity(saccadePoints) == 0;
    assertTrue(isZero);
}

After:

@Test
public void testGetPeakVelocity_aboveThreshold_returnNaN() {
    // ...
    boolean isNaN = Double.isNaN(SaccadeVelocity.getPeakVelocity(saccadePoints));
    assertTrue(isNaN);
}

4. writeCSV.csv - Line Ending Fix

Problem: OpenCSV 5.x defaults to LF (\n) line endings, but the expected test file used CRLF (\r\n), causing FileUtils.contentEquals() to fail.

Fix: Converted src/test/resources/writeCSV.csv from CRLF to LF:

sed -i 's/\r$//' src/test/resources/writeCSV.csv

Test Verification

All 95 tests pass:

$ mvn test -q
# No output (success)

Specific test results:

  • ConvexHullTest: 6/6 passed
  • SaccadeVelocityTest: 6/6 passed
  • DataEntryTest: 5/5 passed

Files Changed

File Change
src/main/java/.../ConvexHull.java Removed (long) casts (3 locations)
src/main/java/.../SaccadeVelocity.java Added foundValidVelocity flag, return NaN when no valid velocities
src/test/java/.../SaccadeVelocityTest.java Updated test to expect NaN
src/test/resources/writeCSV.csv Converted CRLF to LF line endings

Related Issues

These fixes align BEACH-Gaze behavior with TIDE-Gaze (Python implementation) for consistent cross-platform gaze analytics results.

- Remove (long) casts in ConvexHull that truncated floating-point values
- Return NaN instead of 0.0 when all velocities exceed threshold
- Fix writeCSV.csv line endings (CRLF to LF) for OpenCSV 5.x compatibility
@oscararenas12 oscararenas12 merged commit e4ee6df into main Dec 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant