diff --git a/idtap/classes/meter.py b/idtap/classes/meter.py index 76cdabf..5cd9f2d 100644 --- a/idtap/classes/meter.py +++ b/idtap/classes/meter.py @@ -472,6 +472,36 @@ def _hierarchical_position_to_pulse_index(self, positions: List[int], cycle_numb cycle_offset = cycle_number * self._pulses_per_cycle return pulse_index + cycle_offset + def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number: int) -> List[int]: + """Convert pulse index back to hierarchical position (reverse of _hierarchical_position_to_pulse_index).""" + # Remove cycle offset + cycle_offset = cycle_number * self._pulses_per_cycle + within_cycle_index = pulse_index - cycle_offset + + positions = [] + remaining_index = within_cycle_index + + # Work from coarsest to finest level + for level in range(len(self.hierarchy)): + hierarchy_size = self.hierarchy[level] + + if isinstance(hierarchy_size, list): + hierarchy_size = sum(hierarchy_size) + + # Calculate how many pulses are in each group at this level + group_size = self._pulses_per_cycle + for inner_level in range(level + 1): + inner_size = self.hierarchy[inner_level] + if isinstance(inner_size, list): + inner_size = sum(inner_size) + group_size = group_size // inner_size + + position_at_level = remaining_index // group_size + positions.append(position_at_level) + remaining_index = remaining_index % group_size + + return positions + def _calculate_level_start_time(self, positions: List[int], cycle_number: int, reference_level: int) -> float: """Calculate start time of hierarchical unit at reference level.""" @@ -550,7 +580,7 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No total_finest_subdivisions = self._pulses_per_cycle current_group_size = total_finest_subdivisions - for level, size in enumerate(self.hierarchy): + for size in self.hierarchy: if isinstance(size, list): size = sum(size) @@ -572,12 +602,26 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No # 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 + # Bounds checking and hierarchical position correction if current_pulse_index < 0 or current_pulse_index >= len(self.all_pulses): fractional_beat = 0.0 else: current_pulse_time = self.all_pulses[current_pulse_index].real_time + # FIX: If the calculated pulse comes after the query time, find the correct pulse + # This happens when hierarchical position calculation rounds up due to timing variations + if current_pulse_time > real_time: + # Find the pulse that comes at or before the query time + corrected_pulse_index = current_pulse_index + while corrected_pulse_index > 0 and self.all_pulses[corrected_pulse_index].real_time > real_time: + corrected_pulse_index -= 1 + current_pulse_index = corrected_pulse_index + current_pulse_time = self.all_pulses[current_pulse_index].real_time + + # Update positions to reflect the corrected pulse index + # This ensures consistency between hierarchical_position and actual pulse used + positions = self._pulse_index_to_hierarchical_position(current_pulse_index, cycle_number) + # Handle next pulse if current_pulse_index + 1 < len(self.all_pulses): next_pulse_time = self.all_pulses[current_pulse_index + 1].real_time diff --git a/idtap/tests/musical_time_test.py b/idtap/tests/musical_time_test.py index 898b767..402c634 100644 --- a/idtap/tests/musical_time_test.py +++ b/idtap/tests/musical_time_test.py @@ -805,3 +805,42 @@ def test_deep_investigation_of_fractional_beat_calculation(self): print("\nThis test helps identify if the issue is related to position truncation when") print("we're in the middle of subdivisions but reference_level=0 calculation starts") print("from the wrong subdivision boundary.") + + def test_issue_36_hierarchical_position_correction(self): + """Test fix for Issue #36: hierarchical position calculation bug in multi-cycle meters. + + Ensures that the hierarchical position calculation always finds a pulse that comes + at or before the query time, preventing negative fractional_beat values that get + clamped to 0.0. + """ + # Create meter with timing variations that expose the hierarchical calculation bug + meter = Meter(hierarchy=[4, 4, 2], start_time=4.0, tempo=60.0, repetitions=4) + + # Introduce pulse timing variations that can trigger the bug + for i, pulse in enumerate(meter.all_pulses): + if i % 7 == 0: # Every 7th pulse gets adjusted timing + pulse.real_time += 0.05 # 50ms later - enough to cause issues + + # Re-sort pulses by time after timing modifications + meter.pulse_structures[0][0].pulses.sort(key=lambda p: p.real_time) + + # Test times that would previously cause fractional_beat=0.0 due to the bug + test_times = [5.0, 8.5, 12.2, 16.8] + + for time in test_times: + result = meter.get_musical_time(time) + + # Should be within meter boundaries + assert result is not False, f"Time {time} should be within meter boundaries" + + # The fix ensures fractional_beat is reasonable (not clamped to 0.0 due to bug) + assert 0.0 <= result.fractional_beat < 1.0, f"fractional_beat out of range for time {time}" + + # Verify hierarchical position points to correct pulse + pulse_index = meter._hierarchical_position_to_pulse_index( + result.hierarchical_position, result.cycle_number + ) + assert 0 <= pulse_index < len(meter.all_pulses), f"Pulse index should be valid for time {time}" + + pulse_time = meter.all_pulses[pulse_index].real_time + assert pulse_time <= time, f"Calculated pulse should come at/before query time {time}, got {pulse_time}"