fix: correct fractional_beat calculation to use finest level#33
Merged
fix: correct fractional_beat calculation to use finest level#33
Conversation
The fractional_beat should always represent the position within the finest hierarchical level unit (between pulses), regardless of the reference_level parameter. The reference_level only affects hierarchical_position truncation. Fixed examples: - meter.get_musical_time(0.125) now returns C0:0.2+0.000 (was C0:0.2+0.500) - meter.get_musical_time(0.0625) now returns C0:0.1+0.000 (was C0:0.1+0.250) - meter.get_musical_time(0.03125) now returns C0:0.0+0.500 (was C0:0.0+0.125) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
jon-myers
force-pushed
the
fix-fractional-beat
branch
from
d12aadd to
b412df8
Compare
…havior - Fix test timing values to land between pulses when expecting fractional values - Update expected fractional_beat values to match pulse-based calculation - Update test comments to reflect corrected behavior where fractional_beat is always between pulses regardless of reference_level 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
📦 Test Package Built Successfully! This PR has been automatically built and uploaded to TestPyPI for testing. 🔗 TestPyPI Link: https://test.pypi.org/project/idtap/ To test this version: pip install --index-url https://test.pypi.org/simple/ idtap✅ All tests passed and package builds successfully. |
…implementation - Clarify that fractional_beat is ALWAYS calculated between pulses (finest level) - Remove reference level recalculation from Step 5 - Simplify algorithm to remove complex helper functions - Update test case comments to reflect pulse-based calculation - Fix test case expected values to match corrected behavior
Contributor
|
📦 Test Package Built Successfully! This PR has been automatically built and uploaded to TestPyPI for testing. 🔗 TestPyPI Link: https://test.pypi.org/project/idtap/ To test this version: pip install --index-url https://test.pypi.org/simple/ idtap✅ All tests passed and package builds successfully. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixed the fractional_beat calculation to always represent position within the finest hierarchical level unit (between pulses), regardless of the reference_level parameter.
Problem
The previous implementation incorrectly calculated fractional_beat relative to different hierarchical levels depending on the reference_level parameter. This caused confusing results where:
meter.get_musical_time(0.125)returnedC0:0.2+0.500instead ofC0:0.2+0.000meter.get_musical_time(0.0625)returnedC0:0.1+0.250instead ofC0:0.1+0.000meter.get_musical_time(0.03125)returnedC0:0.0+0.125instead ofC0:0.0+0.500Solution
Changes
get_musical_time()to use unified fractional_beat calculationTest Results
🤖 Generated with Claude Code