Skip to content

Fix Issue #38: Resolve cycle boundary failures in get_musical_time() #39

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 10, 2025
Merged

Fix Issue #38: Resolve cycle boundary failures in get_musical_time() #39

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
d5292f6
fix: allow exact end time in get_musical_time() boundary validation (…
jon-myers Sep 10, 2025
3800d48
fix: resolve negative hierarchical positions in pulse correction
jon-myers Sep 10, 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
39 changes: 32 additions & 7 deletions idtap/classes/meter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,12 @@ def _hierarchical_position_to_pulse_index(self, positions: List[int], cycle_numb

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
# Use modulo to get within-cycle index regardless of which cycle the pulse belongs to
within_cycle_index = pulse_index % self._pulses_per_cycle

# Ensure within_cycle_index is non-negative
if within_cycle_index < 0:
within_cycle_index = 0

positions = []
remaining_index = within_cycle_index
Expand All @@ -496,9 +499,9 @@ def _pulse_index_to_hierarchical_position(self, pulse_index: int, cycle_number:
inner_size = sum(inner_size)
group_size = group_size // inner_size

position_at_level = remaining_index // group_size
position_at_level = remaining_index // group_size if group_size > 0 else 0
positions.append(position_at_level)
remaining_index = remaining_index % group_size
remaining_index = remaining_index % group_size if group_size > 0 else 0

return positions

Expand Down Expand Up @@ -561,8 +564,30 @@ def get_musical_time(self, real_time: float, reference_level: Optional[int] = No
if real_time < self.start_time:
return False

end_time = self.start_time + self.repetitions * self.cycle_dur
if real_time >= end_time:
# Calculate proper end time based on actual pulse timing
# For intermediate cycles: use actual next cycle start pulse
# For final cycle: use theoretical calculation (no next cycle exists)
if self.all_pulses and len(self.all_pulses) > 0:
# Calculate which cycle this time would fall into
relative_time = real_time - self.start_time
potential_cycle = int(relative_time // self.cycle_dur)

if potential_cycle < self.repetitions - 1:
# This is an intermediate cycle - use actual next cycle start pulse
next_cycle_first_pulse_index = (potential_cycle + 1) * self._pulses_per_cycle
if next_cycle_first_pulse_index < len(self.all_pulses):
actual_end_time = self.all_pulses[next_cycle_first_pulse_index].real_time
else:
# Fallback to theoretical if pulse doesn't exist
actual_end_time = self.start_time + self.repetitions * self.cycle_dur
else:
# This is the final cycle - use theoretical end time
actual_end_time = self.start_time + self.repetitions * self.cycle_dur
else:
# No pulses available - use theoretical calculation
actual_end_time = self.start_time + self.repetitions * self.cycle_dur

if real_time > actual_end_time:
return False

# Validate reference level
Expand Down
46 changes: 44 additions & 2 deletions idtap/tests/musical_time_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,11 @@ def test_boundary_conditions(self):
result = meter.get_musical_time(end_time - 0.01)
assert result is not False

# At or after end
assert meter.get_musical_time(end_time) is False
# At end (should be valid after Issue #38 fix)
result = meter.get_musical_time(end_time)
assert result is not False, "Exact end time should be valid (Issue #38 fix)"

# After end (should still be invalid)
assert meter.get_musical_time(end_time + 0.01) is False

def test_reference_level_validation(self):
Expand Down Expand Up @@ -844,3 +847,42 @@ def test_issue_36_hierarchical_position_correction(self):

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}"

def test_issue_38_cycle_boundary_failures(self):
"""Test fix for Issue #38: get_musical_time() fails at cycle boundaries.

Ensures that get_musical_time() returns valid musical time objects for
timestamps at exact cycle boundaries, including the final meter boundary.
"""
# Create meter with multiple cycles to test all boundary types
meter = Meter(hierarchy=[4, 4, 2], start_time=0.0, tempo=60.0, repetitions=4)

# Test each cycle boundary including the final one
for cycle in range(meter.repetitions + 1):
boundary_time = meter.start_time + cycle * meter.cycle_dur

result = meter.get_musical_time(boundary_time)

# All boundaries should return valid musical time objects
assert result is not False, f"Cycle boundary at {boundary_time} should return valid musical time"
assert 0.0 <= result.fractional_beat < 1.0, f"fractional_beat should be in valid range for boundary {boundary_time}"

# Boundary should be treated as start of next cycle (if not final boundary)
if cycle < meter.repetitions:
assert result.cycle_number == cycle, f"Boundary {boundary_time} should be in cycle {cycle}"
assert result.hierarchical_position[0] == 0, f"Boundary should be at start of hierarchical position"
else:
# Final boundary - should be treated as start of theoretical next cycle
assert result.cycle_number == cycle, f"Final boundary should indicate cycle {cycle}"

# Test times very close to boundaries to ensure they also work
for cycle in range(meter.repetitions):
boundary_time = meter.start_time + (cycle + 1) * meter.cycle_dur

# Test time just before boundary
near_boundary = boundary_time - 0.001
result = meter.get_musical_time(near_boundary)
assert result is not False, f"Time just before boundary {boundary_time} should be valid"

# Should be in the previous cycle
assert result.cycle_number == cycle, f"Time before boundary should be in cycle {cycle}"
Loading