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

Fix bad log densities #490

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix bad log densities #490

wants to merge 2 commits into from

Conversation

omareldeeb
Copy link

Changes proposed in this pull request

Please provide a detailed description of the changes proposed in this pull
request. This can either be a textual description or a simple list.

In line 629 of beats_hmm.py, the sum of the beats and downbeats observation is used to calculate the log density for non-beat states. These, however, are not guaranteed to define a joint probability density function, which causes the sum to be potentially greater than 1 and the logarithm of a negative number to be used. Since beats are also downbeats, we can simply take the beat observation instead.

If the viterbi algorithm fails to decode a valid path (e.g. as a result of the above error), it returns an empty array, which is indexed in line 305 of downbeats.py, causing an IndexError. To avoid this, we can simply check if the array is empty in line 297.

This pull request fixes #488 #489 .

@superbock
Copy link
Collaborator

This is only partly correct. The output of RNNDownBeatProcessor is actually a probability density function. Thus the situation outlined in #489 should actually not happen — at least not with RNNDownBeatProcessor.

The proposed fix changes the behaviour of DBNDownBeatTrackingProcessor since it actually expects the input to be a probability density function (or at least look like one), i.e. it is designed to have lower beat activations at downbeat positions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IndexError in self.correct branch of DBNDownBeatTrackingProcessor process function.
2 participants