-
Notifications
You must be signed in to change notification settings - Fork 81
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
Adding a new MSA algorithm in MSAF #146
Comments
Hi Alex, thanks for your interest in adding a new algorithm in MSAF! MSAF is going through some updates as of late, mostly thanks to @carlthome. Carl, do you think we should update the way to include new algorithms, or the legacy way should still be functional? For reference, here's the documentation on how to add algorithms to MSAF: https://pythonhosted.org/msaf/algorithms.html#adding-a-new-algorithm-to-msaf |
Wonderful initiative @ax-le! 👏 Think it's absolutely worthwhile to collect algorithms, and collaborate on making sure everything stays comparable. MSAF feels like a good platform for this.
The documented way should be functional. No breaking changes have been introduced, I think, but if I'm mistaken I'll gladly pitch in and resolve any accidental issues!
Sounds good to me! Simply including as_seg in install_requires and calling out to your existing code via a SegmenterInterface implementation would be my first attempt. Porting code is always scary, and nice to avoid. I imagine the version pinning will become immediately problematic however so those would preferably be changed to lower bounds instead. Good news is the dependencies themselves are almost the same as what's already in MSAF (especially the heavy ones like scikit-learn and SciPy), and also including madmom and mirdata sounds good to me. It's of course nice to always try to get rid of unneeded dependencies when possible however (fewer future incompatibility issues to chase down). |
Can see how far the installation gets in #147 🤞 |
Currently stuck on https://github.com/urinieto/msaf/actions/runs/6870581237/job/18685765386#step:5:122 The as-seg release at PyPI specifies sklearn and not scikit-learn. Noticed you've fixed that on https://gitlab.inria.fr/amarmore/autosimilarity_segmentation/-/commit/5ac886b25f716d05d2537f366e5194eb0691f858 but would be good to release that deprecation fix to PyPI too @ax-le! |
This is awesome, thanks @carlthome for taking the initiative on this. Seems like once |
Hi @carlthome , thanks for all the information and the work!
Okay, I'm on it, I will try to find a good trade-off between the requirements of MSAF and of as-seg!
Indeed, thanks for pointing it out! On it also. I will tell you when these changes are pushed in PyPi, should I try to relaunch the install command myself or let you do it? |
@carlthome Done! Note that I had to specify an upper bound on the version of numpy (<1.24), because there was a conflict between scikit-learn version 0.22.2.post1 and numpy version 1.24.4: the use of np.float in some scikit-learn function (called by librosa.effects) lead to an error in numpy (as in this stack overflow issue). Please tell me if I can do anything else! :) |
Great @ax-le! Seems that did the trick, and it's now possible to install as-seg within MSAF: #148 On the NumPy incompatibility, it's been on my radar for a while so thanks for the reminder. New push for resolving that in mir-evaluation/mir_eval#366 now.
The next step would be to call your functions from within a SegmenterInterface. I have a rough idea on how that would look but it's probably best if you try it first since you know the subtleties of your MSA approach the best (I might screw up something crucial, like the 64 sample hop length you've mentioned in your docs, for example). https://pythonhosted.org/msaf/algorithms.html#adding-a-new-algorithm-to-msaf If something doesn't fit right (there's always something with software, isn't it? 😄), let's work through needed tweaks in this thread. Or let me know if you'd prefer that I do a first stab on this and I'll draft a PR. |
Ok perfect, nice to see that the installation went through! :) I will try a first draft in the next days, and I will comment here again when the PR is available! Thanks for the support :) |
Hey!
In short, this could be tweaked in MSAF, but would require time, and it would be more interesting to properly implement these features in MSAF. I don't have claims right now to support the use of Barwise TF matrices, but an article with experimental details is accepted and under publication. I think that it would be highly beneficial to implement Barwise TF matrices, but we might probably need to do it together. So what I propose is 1) that I test the CBM as I have it and make a PR (probably next week), and 2) that we take time to implement Barwise TF matrices. What do you think? |
Closed as it is concerned by PR #153 |
Hi!
I would like to add a MSA algorithm in your MSAF toolbox [1].
I already have a standalone toolbox for it (https://gitlab.inria.fr/amarmore/autosimilarity_segmentation), which is packaged with PyPi, and the easy way would be to add a dependency and encapsulation functions in MSAF.
Still, a more proper way would probably consist of redeveloping everything in the MSAF fashion.
What would would advise me to do?
In particular, would you allow/advise me to implement the easy encaspulation way?
Thanks in advance!
Have a nice day,
Best,
Axel Marmoret.
[1] A. Marmoret, J. E. Cohen and F. Bimbot, "Convolutive Block-Matching Segmentation Algorithm with Application to Music Structure Analysis," 2023 IEEE Workshop on Applications of Signal Processing to Audio and Acoustics (WASPAA), New Paltz, NY, USA, 2023, pp. 1-5, doi: 10.1109/WASPAA58266.2023.10248174.
The text was updated successfully, but these errors were encountered: