-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Summary
While investigating ongoing fractional_beat issues in version 0.1.30, I found a second bug separate from the all_pulses issue that was fixed in PR #35.
The hierarchical position calculation in get_musical_time() sometimes calculates the wrong pulse index, leading to negative time_from_current_pulse values that get clamped to 0.0.
Root Cause Analysis
The Problem
The hierarchical position algorithm calculates positions that sometimes point to a pulse that comes AFTER the query time instead of before/at it. This causes:
time_from_current_pulse = real_time - current_pulse_time
# Becomes negative when current_pulse_time > real_time
fractional_beat = time_from_current_pulse / pulse_duration # Negative!
fractional_beat = max(0.0, min(1.0, fractional_beat)) # Clamped to 0.0Evidence
Testing with transcription 68a3a79fffd9b2d478ee11e8:
Debugging time 5.0:
Calculated pulse index: 7
Current pulse time: 5.060975 ← Pulse AFTER query time
Query time: 5.0
Time from current pulse: -0.060975 ← NEGATIVE!
Raw fractional_beat: -0.441061
Final fractional_beat: 0.000000 ← Clamped to 0
Debugging time 8.29:
Calculated pulse index: 32
Current pulse time: 8.349985 ← Pulse AFTER query time
Query time: 8.29
Time from current pulse: -0.059985 ← NEGATIVE!
Final fractional_beat: 0.000000 ← Clamped to 0
Working cases find pulses that come before the query time:
Debugging time 4.19:
Current pulse time: 4.093249 ← Pulse BEFORE query time
Query time: 4.19
Time from current pulse: 0.096751 ← POSITIVE ✓
Final fractional_beat: 0.699846 ← Correct!
Impact
- Multi-cycle meters return
fractional_beat=0.0for many time points - Trajectory curve visualization shows clustering instead of smooth curves
- Affects musical analysis that depends on sub-pulse positioning
Location
The bug is in the hierarchical position calculation logic in get_musical_time() around lines 600-670 in meter.py.
Reproduction
from idtap import SwaraClient, Piece
client = SwaraClient()
piece = Piece.from_json(client.get_piece("68a3a79fffd9b2d478ee11e8"))
meter = piece.meters[0]
# These should have non-zero fractional_beat but return 0.0:
result1 = meter.get_musical_time(5.0) # Returns fractional_beat=0.0 (BUG)
result2 = meter.get_musical_time(8.29) # Returns fractional_beat=0.0 (BUG)
# These work correctly:
result3 = meter.get_musical_time(4.19) # Returns fractional_beat=0.699846 ✓
result4 = meter.get_musical_time(6.0) # Returns fractional_beat=0.322064 ✓Required Fix
The hierarchical position calculation needs to be corrected to always find the pulse that comes at or before the query time, not after it. This likely involves:
- Fixing the subdivision duration calculations
- Ensuring proper floor division vs ceiling division
- Adding comprehensive multi-cycle tests to catch this class of bug
Testing Needed
Current tests don't cover multi-cycle scenarios adequately. Need tests that:
- Test fractional_beat across all cycles in a multi-cycle meter
- Verify continuous fractional_beat progression within cycles
- Test edge cases at cycle boundaries
- Test various hierarchy structures across multiple cycles
This bug affects real-world usage where transcriptions span multiple meter cycles, making trajectory analysis unreliable.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com