Skip to content
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.

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

wrong values in metrical strength features #363

Closed
sildater opened this issue Jul 24, 2024 · 2 comments · Fixed by #364
Closed

wrong values in metrical strength features #363

sildater opened this issue Jul 24, 2024 · 2 comments · Fixed by #364
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@sildater
Copy link
Member

the metrical strength features in note_features.py return four attributes:

"beat_phase",
"metrical_strength_downbeat",
"metrical_strength_secondary",
"metrical_strength_weak"

metrical_strength_downbeat and metrical_strength_weak return 1.0 in the presence of a downbeat which seems wrong for the latter, line 1066 is probably the problem. A bit of documentation on each feature is also missing (what is secondary, what is weak, does it work for triple meters?).

@sildater sildater added bug Something isn't working invalid This doesn't seem right labels Jul 24, 2024
@manoskary
Copy link
Member

Hello, thanks for the issue. These features are taken from the basis functions of the basis mixer. Some documentation that could always be useful, I can look into it. But maybe @neosatrapahereje would be the best person to write the documentation for that or review the resulting PR.

@sildater
Copy link
Member Author

I could also fix, I already know where it is.

@sildater sildater linked a pull request Jul 25, 2024 that will close this issue
@sildater sildater closed this as completed Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants