diff --git a/docs/musical-time-spec.md b/docs/musical-time-spec.md index e0928fa..185fc4b 100644 --- a/docs/musical-time-spec.md +++ b/docs/musical-time-spec.md @@ -56,11 +56,12 @@ 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 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) +- `fractionalBeat`: ALWAYS represents fractional position between pulses (finest level), regardless of referenceLevel +- `referenceLevel`: Only affects the truncation of `hierarchicalPosition`: + - `referenceLevel=0`: hierarchicalPosition includes only beat level + - `referenceLevel=1`: hierarchicalPosition includes beat and subdivision levels + - `referenceLevel=n`: hierarchicalPosition includes levels 0 through n + - Default (None): hierarchicalPosition includes all levels **Boundaries:** - **Start**: `realTime >= meter.startTime` @@ -107,63 +108,51 @@ for each level in hierarchy: remainingTime = remainingTime % subdivisionDuration ``` -### Step 4: Fractional Beat Calculation (Level-Based) +### Step 4: Fractional Beat Calculation (Always Pulse-Based) -The fractional beat calculation depends on the specified reference level: - -#### Default Behavior (Pulse-Based) -When `referenceLevel` is not specified or equals `hierarchy.length - 1`, calculate fraction between pulses: +The fractional beat ALWAYS represents the position between pulses (finest level), regardless of reference level: ``` currentPulseIndex = hierarchicalPositionToPulseIndex(positions, cycleNumber) -currentPulseTime = meter.allPulses[currentPulseIndex].realTime - -// Handle next pulse (accounting for cycle boundaries) -if currentPulseIndex + 1 < meter.allPulses.length: - nextPulseTime = meter.allPulses[currentPulseIndex + 1].realTime -else: - // Last pulse - use next cycle start - nextCycleStart = meter.startTime + (cycleNumber + 1) * meter.cycleDur - nextPulseTime = nextCycleStart -pulseDuration = nextPulseTime - currentPulseTime -if pulseDuration <= 0: +// Bounds checking +if currentPulseIndex < 0 or currentPulseIndex >= meter.allPulses.length: fractionalBeat = 0.0 else: - timeFromCurrentPulse = realTime - currentPulseTime - fractionalBeat = timeFromCurrentPulse / pulseDuration + currentPulseTime = meter.allPulses[currentPulseIndex].realTime + + // Handle next pulse (accounting for cycle boundaries) + if currentPulseIndex + 1 < meter.allPulses.length: + nextPulseTime = meter.allPulses[currentPulseIndex + 1].realTime + else: + // Last pulse - use next cycle start + nextCycleStart = meter.startTime + (cycleNumber + 1) * meter.cycleDur + nextPulseTime = nextCycleStart + + pulseDuration = nextPulseTime - currentPulseTime + if pulseDuration <= 0: + fractionalBeat = 0.0 + else: + timeFromCurrentPulse = realTime - currentPulseTime + fractionalBeat = timeFromCurrentPulse / pulseDuration // Clamp to [0, 1] range fractionalBeat = max(0.0, min(1.0, fractionalBeat)) ``` -#### Reference Level Behavior -When `referenceLevel` is specified and < `hierarchy.length - 1`: - -``` -// Truncate hierarchical position to reference level + 1 -truncatedPosition = positions[0..referenceLevel] - -// Calculate start time of current reference-level unit -currentLevelStartTime = calculateLevelStartTime(truncatedPosition, cycleNumber, referenceLevel) +### Step 5: Handle Reference Level Truncation -// Calculate duration of reference-level unit (accounting for actual pulse timing) -levelDuration = calculateLevelDuration(truncatedPosition, cycleNumber, referenceLevel) +If a reference level is specified, truncate the hierarchical position (fractional_beat remains unchanged): -if levelDuration <= 0: - fractionalBeat = 0.0 -else: - timeFromLevelStart = realTime - currentLevelStartTime - fractionalBeat = timeFromLevelStart / levelDuration - -// Clamp to [0, 1] range -fractionalBeat = max(0.0, min(1.0, fractionalBeat)) - -// Update hierarchical position to only include levels up to reference -positions = truncatedPosition +``` +if referenceLevel is not None and referenceLevel < len(hierarchy): + // Truncate positions to reference level for final result + positions = positions[0:referenceLevel+1] + +// Note: fractionalBeat is NOT recalculated - it always remains pulse-based ``` -### Step 5: Result Construction +### Step 6: Result Construction ``` return MusicalTime { cycleNumber: cycleNumber, @@ -201,64 +190,7 @@ cycleOffset = cycleNumber * meter.getPulsesPerCycle() return pulseIndex + cycleOffset ``` -### calculateLevelStartTime() - -**Purpose:** Calculate the start time of a hierarchical unit at a given reference level. - -**Signature:** -``` -calculateLevelStartTime(positions: number[], cycleNumber: number, referenceLevel: number): number -``` - -**Algorithm:** -``` -// Find the pulse index for the start of this reference-level unit -startPositions = positions.slice(0, referenceLevel + 1) -// Zero out all positions below the reference level -for i = referenceLevel + 1 to hierarchy.length - 1: - startPositions[i] = 0 - -startPulseIndex = hierarchicalPositionToPulseIndex(startPositions, cycleNumber) -return meter.allPulses[startPulseIndex].realTime -``` - -### calculateLevelDuration() - -**Purpose:** Calculate the actual duration of a hierarchical unit based on pulse timing. - -**Signature:** -``` -calculateLevelDuration(positions: number[], cycleNumber: number, referenceLevel: number): number -``` - -**Algorithm:** -``` -// Get start time of current unit -startTime = calculateLevelStartTime(positions, cycleNumber, referenceLevel) - -// Calculate start time of next unit at same level -nextPositions = positions.slice() -nextPositions[referenceLevel]++ - -// Handle overflow - if we've exceeded this level, move to next cycle or higher level -if nextPositions[referenceLevel] >= hierarchy[referenceLevel]: - if referenceLevel == 0: - // Next beat is in next cycle - nextCycleNumber = cycleNumber + 1 - if nextCycleNumber >= meter.repetitions: - // Use meter end time - return meter.startTime + meter.repetitions * meter.cycleDur - startTime - nextPositions[0] = 0 - return calculateLevelStartTime(nextPositions, nextCycleNumber, referenceLevel) - startTime - else: - // Carry over to higher level - nextPositions[referenceLevel] = 0 - nextPositions[referenceLevel - 1]++ - return calculateLevelDuration(nextPositions, cycleNumber, referenceLevel - 1) - -endTime = calculateLevelStartTime(nextPositions, cycleNumber, referenceLevel) -return endTime - startTime -``` +**Note:** The simplified implementation no longer requires the complex `calculateLevelStartTime()` and `calculateLevelDuration()` helper functions that were used in the previous reference-level-based fractional beat calculation. ## Edge Cases & Error Handling @@ -299,10 +231,10 @@ Query: getMusicalTime(2.375, referenceLevel=0) Expected: - cycleNumber: 0 -- hierarchicalPosition: [2] (Beat 3) -- 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" +- hierarchicalPosition: [2] (Beat 3 only, due to referenceLevel=0) +- fractionalBeat: 0.0 (exactly on pulse, fractionalBeat always pulse-based) +- toString(): "C0:2+0.000" +- Readable: "Cycle 1: Beat 3" ``` ### Test Case 3: Reference Level - Subdivision Level (referenceLevel=1) @@ -313,9 +245,9 @@ Query: getMusicalTime(2.375, referenceLevel=1) Expected: - cycleNumber: 0 - hierarchicalPosition: [2, 1] (Beat 3, Subdivision 2) -- 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" +- fractionalBeat: 0.0 (exactly on pulse, fractionalBeat always pulse-based) +- toString(): "C0:2.1+0.000" +- Readable: "Cycle 1: Beat 3, Subdivision 2" ``` ### Test Case 4: Complex Hierarchy with Reference Levels @@ -326,7 +258,7 @@ Query: getMusicalTime(0.15625, referenceLevel=1) Expected: - cycleNumber: 0 - hierarchicalPosition: [1, 0] (Beat 2, Subdivision 1) -- fractionalBeat: 0.25 (0.25 through subdivision duration) +- fractionalBeat: 0.25 (0.25 through finest-level pulse duration) - toString(): "C0:1.0+0.250" ``` diff --git a/idtap/classes/meter.py b/idtap/classes/meter.py index 03ab853..d42735b 100644 --- a/idtap/classes/meter.py +++ b/idtap/classes/meter.py @@ -621,99 +621,40 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No remaining_time = remaining_time % subdivision_duration # Step 4: Fractional beat calculation - if ref_level == len(self.hierarchy) - 1: - # 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: - # 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)) - + # ALWAYS calculate fractional_beat as position within finest level unit (between pulses) + # This is independent of reference_level, which only affects hierarchical_position truncation + current_pulse_index = self._hierarchical_position_to_pulse_index(positions, cycle_number) + + # Bounds checking + if current_pulse_index < 0 or current_pulse_index >= len(self.all_pulses): + fractional_beat = 0.0 else: - # Reference level behavior - truncated_positions = positions[:ref_level + 1] + current_pulse_time = self.all_pulses[current_pulse_index].real_time - # 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 + # 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: - # 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) + # Last pulse - use next cycle start + next_cycle_start = self.start_time + (cycle_number + 1) * self.cycle_dur + next_pulse_time = next_cycle_start - if level_duration <= 0: + pulse_duration = next_pulse_time - current_pulse_time + if pulse_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)) - - # Update positions to only include levels up to reference for final result - positions = truncated_positions + time_from_current_pulse = real_time - current_pulse_time + fractional_beat = time_from_current_pulse / pulse_duration + + # Clamp to [0, 1] range + fractional_beat = max(0.0, min(1.0, fractional_beat)) + + # Step 5: Handle reference level truncation (if specified) + if ref_level is not None and ref_level < len(self.hierarchy) - 1: + # Truncate positions to reference level for final result + positions = positions[:ref_level + 1] - # Step 5: Result construction + # Step 6: Result construction return MusicalTime( cycle_number=cycle_number, hierarchical_position=positions, diff --git a/idtap/tests/musical_time_test.py b/idtap/tests/musical_time_test.py index 155fbbf..12e0d38 100644 --- a/idtap/tests/musical_time_test.py +++ b/idtap/tests/musical_time_test.py @@ -81,7 +81,7 @@ def test_regular_meter_default_level(self): """Test Case 1 from spec: Regular meter with default level.""" meter = Meter(hierarchy=[4, 4], tempo=240, start_time=0, repetitions=3) # Extended to 3 repetitions - result = meter.get_musical_time(2.375) # This is now in the 3rd cycle + result = meter.get_musical_time(2.40625) # Halfway between subdivision 2 and 3 in 3rd cycle assert result is not False assert result.cycle_number == 2 # Third cycle (0-indexed) @@ -93,26 +93,46 @@ 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 = 37.5% through cycle + result = meter.get_musical_time(1.375, reference_level=0) # 1.375s = 2nd cycle, beat 1, subdivision 2 (exactly on pulse) 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.375) < 0.01 # 37.5% through cycle (ref_level=0 = within cycle) - assert "C1:1+" in str(result) + assert result.hierarchical_position == [1] # Only beat level (beat 2) due to reference_level=0 + assert abs(result.fractional_beat - 0.0) < 0.01 # Exactly on pulse (fractional_beat always pulse-based) + assert str(result) == "C1:1+0.000" 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 2, halfway through beat + result = meter.get_musical_time(0.40625, reference_level=1) # Beat 1, subdivision 2, halfway between pulses 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.5) < 0.01 # 50% through beat (ref_level=1 = within beat) + assert abs(result.fractional_beat - 0.5) < 0.01 # 50% between pulses (fractional_beat always pulse-based) assert str(result) == "C0:1.2+0.500" + def test_johns_specific_examples(self): + """Test Jon's specific examples that revealed the fractional_beat calculation issue.""" + meter = Meter(hierarchy=[4, 4], tempo=240, start_time=0, repetitions=3) + + # These examples should work correctly after the fix + result = meter.get_musical_time(0.5) + assert str(result) == "C0:2.0+0.000", f"Expected C0:2.0+0.000, got {result}" + + result = meter.get_musical_time(0.25) + assert str(result) == "C0:1.0+0.000", f"Expected C0:1.0+0.000, got {result}" + + result = meter.get_musical_time(0.125) + assert str(result) == "C0:0.2+0.000", f"Expected C0:0.2+0.000, got {result}" + + result = meter.get_musical_time(0.0625) + assert str(result) == "C0:0.1+0.000", f"Expected C0:0.1+0.000, got {result}" + + result = meter.get_musical_time(0.03125) + assert str(result) == "C0:0.0+0.500", f"Expected C0:0.0+0.500, got {result}" + def test_complex_hierarchy(self): """Test Case 4 from spec: Complex hierarchy with reference levels.""" meter = Meter(hierarchy=[3, 2, 4], tempo=480, start_time=0, repetitions=1) # Slower tempo @@ -328,7 +348,7 @@ def test_recursive_overflow_edge_case(self): result = meter.get_musical_time(time_at_subdivision_boundary, reference_level=0) assert result is not False assert result.beat == 0 - assert abs(result.fractional_beat - 0.4995) < 0.001 # 49.95% through cycle (ref_level=0 = within cycle) + assert abs(result.fractional_beat - 0.997) < 0.01 # 99.7% between pulses (fractional_beat always pulse-based) # Same time with subdivision reference should handle overflow correctly result = meter.get_musical_time(time_at_subdivision_boundary, reference_level=1)