Fix fractional_beat clustering with sparse pulse data (Issue #28)#30
Merged
Fix fractional_beat clustering with sparse pulse data (Issue #28)#30
Conversation
…sue #28) **Root Cause Analysis:** The fractional_beat clustering issue was caused by sparse pulse data in real transcription meters. While synthetic meters have complete pulse arrays (32/32 pulses), real transcription data contains only manually annotated pulses (2/32 pulses), breaking pulse-based timing calculations. **Key Discovery:** - Real meters: _pulses_per_cycle=32, all_pulses count=2 (6% of expected!) - Synthetic meters: _pulses_per_cycle=32, all_pulses count=32 (100% complete) - Previous fix using full positions was ineffective - didn't address pulse sparsity **Solution:** 1. **Detection**: Check if pulse count < 50% of expected pulses 2. **Fallback**: Use proportional cycle timing instead of broken pulse indexing 3. **Preservation**: Maintain existing pulse-based functionality for complete data **New Methods:** - `_calculate_proportional_level_start_time()`: Beat boundaries via cycle division - `_calculate_proportional_level_duration()`: Unit durations via hierarchical ratios **Results Validation:** - Real data fractional_beat: 0.000-0.026 → 0.104-0.881 ✅ - Synthetic data: unchanged (0.104-0.881) ✅ - Near-identical values between real and synthetic meters ✅ - All 29 tests passing with no regressions ✅ This fix ensures musical visualizations properly distribute events across beat durations instead of clustering them at beat boundaries, resolving the core issue described in GitHub Issue #28. 🤖 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. |
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
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
COMPREHENSIVE SOLUTION: Resolves Issue #28 by implementing proportional timing fallback for sparse pulse data in real transcription meters, ensuring fractional_beat values properly distribute 0.0-1.0 within beats.
Root Cause Analysis
The clustering issue was not in position truncation logic, but in the fundamental pulse data difference:
Real Transcription Meters (Broken)
_pulses_per_cycle=32,_pulse_dur=0.128s✅all_pulses count=2❌ (only 6% of expected pulses!)Synthetic Meters (Working)
_pulses_per_cycle=32,_pulse_dur=0.128s✅all_pulses count=32✅ (100% complete pulse grid)Solution Implementation
1. Sparse Pulse Detection
2. Proportional Timing Methods
_calculate_proportional_level_start_time(): Uses cycle division instead of pulse indexing_calculate_proportional_level_duration(): Uses hierarchical ratios instead of pulse differences3. Intelligent Fallback Strategy
Results Comparison
Before Fix (Broken Clustering)
After Fix (Proper Distribution)
Consistency Verification
Testing Results
Impact
Backward Compatibility
This fix is fully backward compatible:
🤖 Generated with Claude Code