librosa / librosa

Python library for audio and music analysis
https://librosa.org/
ISC License
7.04k stars 959 forks source link

cross-similarity function #770

Closed rabitt closed 5 years ago

rabitt commented 5 years ago

I'd love to have a version of librosa.segment.recurrence_matrix that compares two arbitrary sequences. A lot of the code in the recurrence_matrix function will extend to the two sequence case, happy to make a PR. Any suggestions on how such a function should work within the library?

bmcfee commented 5 years ago

This sounds like a good feature to have! I guess the first question is whether this would be better done as a separate function, or by extending recurrence_matrix.

Arguments for separate function

Arguments for the same function

Verdict?

I'm leaning toward a separate function cross_similarity, which would otherwise look a lot like recurrence_matrix internally. But I'm open to counter-arguments. Anyone else want to chime in?

rabitt commented 5 years ago

@bmcfee What about abstracting the common bits into a separate helper function, and then making two separate functions, cross_similarity and a simplification of recurrence_matrix that rely on the helper?

bmcfee commented 5 years ago

@rabitt that also sounds reasonable. Maybe it's worth prototyping an implementation of cross_similarity first and then factoring out the common bits?

rabitt commented 5 years ago

recurrence_matrix has an axis parameter - in the cross_similarity case, do you think it's reasonable to force the data to be in a consistent shape, e.g. (n_features, n_times) to avoid having multiple axis parameters?

bmcfee commented 5 years ago

yeah, there's precedent for that in dtw.

rabitt commented 5 years ago

(asking questions as I implement :) ) - I guess we want the knn classifier to be relative to (i.e. fit to) the features in the first sequence, and then the connections are built from the features in the second sequence? Should we make this more flexible?

bmcfee commented 5 years ago

I guess we want the knn classifier to be relative to (i.e. fit to) the features in the first sequence, and then the connections are built from the features in the second sequence? Should we make this more flexible?

The NearestNeighbors object is a data structure, not a classifier. If you have two arrays X1 and X2, of durations n and m respectively, the output of cross_similarity should have shape (n, m). The entries (i, j) should correspond to X1[:, i] being a nearest neighbor of X2[:, j]. So you should fit on X1 and predict on X2, if that makes sense?

bmcfee commented 5 years ago

Fixed by merging #904