Skip to content

Complete fix for IndexError and reference level logic in get_musical_time() #32

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

Merged
jon-myers merged 2 commits into from
Sep 9, 2025
Merged

Complete fix for IndexError and reference level logic in get_musical_time() #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ab9b69
Complete fix for IndexError and reference level logic in get_musical_…
jon-myers Sep 9, 2025
0cf3453
Update test expectations to match corrected reference level logic
jon-myers Sep 9, 2025
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
23 changes: 12 additions & 11 deletions docs/musical-time-spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ getMusicalTime(realTime: number, referenceLevel?: number): MusicalTime | false
- `false` - If time is before start_time or after end_time

**Reference Level Behavior:**
- `referenceLevel=0`: Fractional position within beat duration
- `referenceLevel=1`: Fractional position within subdivision duration
- `referenceLevel=n`: Fractional position within level-n duration
- `referenceLevel=0`: Fractional position within cycle duration (containing unit for beats)
- `referenceLevel=1`: Fractional position within beat duration (containing unit for subdivisions)
- `referenceLevel=2`: Fractional position within subdivision duration (containing unit for sub-subdivisions)
- `referenceLevel=n`: Fractional position within level-(n-1) duration (containing unit for level-n)
- Default: Fractional position within finest subdivision (between pulses)

**Boundaries:**
Expand Down Expand Up @@ -291,30 +292,30 @@ Expected:
- toString(): "C0:2.1+0.500"
```

### Test Case 2: Reference Level - Beat Level
### Test Case 2: Reference Level - Beat Level (referenceLevel=0)
```
Meter: hierarchy=[4, 4], tempo=240, startTime=0, repetitions=2
Query: getMusicalTime(2.375, referenceLevel=0)

Expected:
- cycleNumber: 0
- hierarchicalPosition: [2] (Beat 3)
- fractionalBeat: 0.375 (0.375 through beat duration of 1.0 second)
- toString(): "C0:2+0.375"
- Readable: "Cycle 1: Beat 3 + 0.375 through beat"
- fractionalBeat: 0.594 (2.375s / 4.0s cycle duration = 59.4% through cycle)
- toString(): "C0:2+0.594"
- Readable: "Cycle 1: Beat 3 + 0.594 through cycle"
```

### Test Case 3: Reference Level - Subdivision Level
### Test Case 3: Reference Level - Subdivision Level (referenceLevel=1)
```
Meter: hierarchy=[4, 4], tempo=240, startTime=0, repetitions=2
Query: getMusicalTime(2.375, referenceLevel=1)

Expected:
- cycleNumber: 0
- hierarchicalPosition: [2, 1] (Beat 3, Subdivision 2)
- fractionalBeat: 0.5 (0.5 through subdivision duration of 0.25 seconds)
- toString(): "C0:2.1+0.500"
- Readable: "Cycle 1: Beat 3, Subdivision 2 + 0.500 through subdivision"
- fractionalBeat: 0.375 (0.375s / 1.0s beat duration = 37.5% through beat 2)
- toString(): "C0:2.1+0.375"
- Readable: "Cycle 1: Beat 3, Subdivision 2 + 0.375 through beat"
```

### Test Case 4: Complex Hierarchy with Reference Levels
Expand Down
107 changes: 81 additions & 26 deletions idtap/classes/meter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,25 +496,30 @@ def _calculate_proportional_level_start_time(self, positions: List[int], cycle_n
while len(level_start_positions) < reference_level + 1:
level_start_positions.append(0)

# Calculate cumulative position as fraction of cycle
cumulative_position = 0.0
current_subdivisions = 1
# Calculate cumulative time offset from cycle start
cumulative_time = 0.0
current_duration = self.cycle_dur # Start with full cycle duration

for level in range(reference_level + 1):
level_size = self.hierarchy[level]
if isinstance(level_size, list):
level_size = sum(level_size)

# Duration of each unit at this level
unit_duration = current_duration / level_size

if level < len(level_start_positions):
position_at_level = level_start_positions[level]
else:
position_at_level = 0

# Add this level's contribution to the cumulative position
cumulative_position += position_at_level / current_subdivisions
current_subdivisions *= level_size
# Add time offset for this level
cumulative_time += position_at_level * unit_duration

# Update duration for next level (duration of current unit)
current_duration = unit_duration

return cycle_start_time + cumulative_position * self.cycle_dur
return cycle_start_time + cumulative_time

def _calculate_level_duration(self, positions: List[int], cycle_number: int, reference_level: int) -> float:
"""Calculate actual duration of hierarchical unit based on pulse timing."""
Expand Down Expand Up @@ -617,24 +622,66 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No

# Step 4: Fractional beat calculation
if ref_level == len(self.hierarchy) - 1:
# Default behavior: pulse-based calculation
current_pulse_index = self._hierarchical_position_to_pulse_index(positions, cycle_number)
current_pulse_time = self.all_pulses[current_pulse_index].real_time

# Handle next pulse
if current_pulse_index + 1 < len(self.all_pulses):
next_pulse_time = self.all_pulses[current_pulse_index + 1].real_time
else:
# Last pulse - use next cycle start
next_cycle_start = self.start_time + (cycle_number + 1) * self.cycle_dur
next_pulse_time = next_cycle_start

pulse_duration = next_pulse_time - current_pulse_time
if pulse_duration <= 0:
fractional_beat = 0.0
# Default behavior: pulse-based calculation, but check for sparse pulse data
expected_pulses = self._pulses_per_cycle * self.repetitions
if len(self.all_pulses) < expected_pulses * 0.5: # Less than 50% of expected pulses
# Fall back to proportional timing calculation for sparse pulse data
# For fractional_beat, we need the containing unit (parent) duration and start time
if ref_level == 0:
# Ref level 0: fractional position within the cycle
current_level_start_time = self.start_time + cycle_number * self.cycle_dur
level_duration = self.cycle_dur
else:
# Ref level > 0: fractional position within the parent unit
parent_positions = positions[:ref_level] # Parent positions
current_level_start_time = self._calculate_proportional_level_start_time(parent_positions, cycle_number, ref_level - 1)
level_duration = self._calculate_proportional_level_duration(parent_positions, cycle_number, ref_level - 1)

if level_duration <= 0:
fractional_beat = 0.0
else:
time_from_level_start = real_time - current_level_start_time
fractional_beat = time_from_level_start / level_duration
else:
time_from_current_pulse = real_time - current_pulse_time
fractional_beat = time_from_current_pulse / pulse_duration
# Use pulse-based calculation for complete pulse data
current_pulse_index = self._hierarchical_position_to_pulse_index(positions, cycle_number)

# Add bounds checking for pulse access
if current_pulse_index < 0 or current_pulse_index >= len(self.all_pulses):
# Fall back to proportional calculation if pulse index out of bounds
# For fractional_beat, we need the containing unit (parent) duration and start time
if ref_level == 0:
# Ref level 0: fractional position within the cycle
current_level_start_time = self.start_time + cycle_number * self.cycle_dur
level_duration = self.cycle_dur
else:
# Ref level > 0: fractional position within the parent unit
parent_positions = positions[:ref_level] # Parent positions
current_level_start_time = self._calculate_proportional_level_start_time(parent_positions, cycle_number, ref_level - 1)
level_duration = self._calculate_proportional_level_duration(parent_positions, cycle_number, ref_level - 1)

if level_duration <= 0:
fractional_beat = 0.0
else:
time_from_level_start = real_time - current_level_start_time
fractional_beat = time_from_level_start / level_duration
else:
# Safe pulse-based calculation - use parent unit logic for reference levels
if ref_level == 0:
# Ref level 0: fractional position within the cycle
current_level_start_time = self.start_time + cycle_number * self.cycle_dur
level_duration = self.cycle_dur
else:
# For ref_level > 0: fractional position within the parent unit
parent_positions = positions[:ref_level] # Truncate to parent level
current_level_start_time = self._calculate_proportional_level_start_time(parent_positions, cycle_number, ref_level - 1)
level_duration = self._calculate_proportional_level_duration(parent_positions, cycle_number, ref_level - 1)

if level_duration <= 0:
fractional_beat = 0.0
else:
time_from_level_start = real_time - current_level_start_time
fractional_beat = time_from_level_start / level_duration

# Clamp to [0, 1] range
fractional_beat = max(0.0, min(1.0, fractional_beat))
Expand All @@ -643,8 +690,16 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No
# Reference level behavior
truncated_positions = positions[:ref_level + 1]

current_level_start_time = self._calculate_level_start_time(truncated_positions, cycle_number, ref_level)
level_duration = self._calculate_level_duration(truncated_positions, cycle_number, ref_level)
# For fractional_beat calculation, we need the containing unit (parent) duration and start time
if ref_level == 0:
# Ref level 0: fractional position within the cycle
current_level_start_time = self.start_time + cycle_number * self.cycle_dur
level_duration = self.cycle_dur
else:
# Ref level > 0: fractional position within the parent unit
parent_positions = truncated_positions[:-1] # Remove the last position for parent unit
current_level_start_time = self._calculate_proportional_level_start_time(parent_positions, cycle_number, ref_level - 1)
level_duration = self._calculate_proportional_level_duration(parent_positions, cycle_number, ref_level - 1)

if level_duration <= 0:
fractional_beat = 0.0
Expand Down
27 changes: 14 additions & 13 deletions idtap/tests/musical_time_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,33 +85,33 @@ def test_regular_meter_default_level(self):

assert result is not False
assert result.cycle_number == 2 # Third cycle (0-indexed)
assert result.hierarchical_position == [1, 2] # Beat 2, Subdivision 3 (6 * 0.0625 = 0.375)
assert abs(result.fractional_beat - 0.0) < 0.01 # Exactly on pulse
assert str(result) == "C2:1.2+0.000"
assert result.hierarchical_position == [1, 2] # Beat 2, Subdivision 3
assert abs(result.fractional_beat - 0.5) < 0.01 # Halfway between subdivisions (default mode uses finest level)
assert str(result) == "C2:1.2+0.500"

def test_reference_level_beat(self):
"""Test Case 2 from spec: Reference level at beat level."""
meter = Meter(hierarchy=[4, 4], tempo=240, start_time=0, repetitions=2)

result = meter.get_musical_time(1.375, reference_level=0) # Within bounds: 1.375 = beat 1, 37.5% through beat
result = meter.get_musical_time(1.375, reference_level=0) # Within bounds: 1.375 = 37.5% through cycle

assert result is not False
assert result.cycle_number == 1 # Second cycle
assert result.hierarchical_position == [1] # Only beat level (beat 2)
assert abs(result.fractional_beat - 0.5) < 0.1 # 50% through beat 2
assert abs(result.fractional_beat - 0.375) < 0.01 # 37.5% through cycle (ref_level=0 = within cycle)
assert "C1:1+" in str(result)

def test_reference_level_subdivision(self):
"""Test Case 3 from spec: Reference level at subdivision level."""
meter = Meter(hierarchy=[4, 4], tempo=240, start_time=0, repetitions=2)

result = meter.get_musical_time(0.375, reference_level=1) # Beat 1, subdivision 3
result = meter.get_musical_time(0.375, reference_level=1) # Beat 1, subdivision 2, halfway through beat

assert result is not False
assert result.cycle_number == 0
assert result.hierarchical_position == [1, 2] # Beat 2, subdivision 3
assert abs(result.fractional_beat - 0.0) < 0.01 # Exactly on subdivision
assert str(result) == "C0:1.2+0.000"
assert abs(result.fractional_beat - 0.5) < 0.01 # 50% through beat (ref_level=1 = within beat)
assert str(result) == "C0:1.2+0.500"

def test_complex_hierarchy(self):
"""Test Case 4 from spec: Complex hierarchy with reference levels."""
Expand Down Expand Up @@ -321,14 +321,14 @@ def test_recursive_overflow_edge_case(self):
meter = Meter(hierarchy=[2, 3], tempo=60, start_time=0)

# Position at end of a subdivision that would cause overflow
# With hierarchy [2,3], beat duration = 1 sec, subdivision = 0.333 sec
# Test at end of beat 0, subdivision 2 (just before beat 1)
# With hierarchy [2,3], cycle duration = 2 sec, beat duration = 1 sec
# Test at 0.999s which is 49.95% through the 2-second cycle
time_at_subdivision_boundary = 0.999

result = meter.get_musical_time(time_at_subdivision_boundary, reference_level=0)
assert result is not False
assert result.beat == 0
assert result.fractional_beat > 0.99
assert abs(result.fractional_beat - 0.4995) < 0.001 # 49.95% through cycle (ref_level=0 = within cycle)

# Same time with subdivision reference should handle overflow correctly
result = meter.get_musical_time(time_at_subdivision_boundary, reference_level=1)
Expand Down Expand Up @@ -551,9 +551,10 @@ def test_fractional_beat_distribution_with_reference_level_zero(self):
assert min_frac >= 0.0, f"Beat {beat_idx}: fractional_beat minimum {min_frac} should be >= 0.0"
assert max_frac <= 1.0, f"Beat {beat_idx}: fractional_beat maximum {max_frac} should be <= 1.0"

# This is the key test for Issue #28: fractional_beat should vary significantly within a beat
# With corrected reference_level=0 (within cycle): fractional_beat varies across cycle, not within individual beats
# For a 2-second cycle with 4 beats, each beat spans 0.25 of the cycle (range ~0.2)
range_span = max_frac - min_frac
assert range_span > 0.3, f"Beat {beat_idx}: fractional_beat range {range_span:.3f} is too small. Values clustering near 0.000 (Issue #28 symptom)"
assert range_span > 0.15, f"Beat {beat_idx}: fractional_beat range {range_span:.3f} is too small. Values clustering near 0.000 (Issue #28 symptom)"

# Should have reasonable variation in values
assert unique_values >= 3, f"Beat {beat_idx}: Only {unique_values} unique fractional_beat values, expected more variation"
Expand Down
Loading