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

Move BSS eval to own project? #22

Open
hagenw opened this issue Jul 30, 2018 · 10 comments
Open

Move BSS eval to own project? #22

hagenw opened this issue Jul 30, 2018 · 10 comments

Comments

@hagenw
Copy link
Contributor

hagenw commented Jul 30, 2018

After the end of SiSEC 2018, this repository might be slightly confusing as it mixes the SiSEC related evaluation with the BSS eval toolkit. I'm pretty sure BSS eval will be further used and developed, so it might be a good idea to create a related BSS eval repository and python package that just contains BSS eval.

@faroit
Copy link
Member

faroit commented Jul 30, 2018

yes, this absolutely makes sense. Our plan was to add this to mir_eval (mir-evaluation/mir_eval#272). But we also considered a separate repository. You have some opinion on this?

@hagenw
Copy link
Contributor Author

hagenw commented Jul 31, 2018

Good point, considering the effort they spent at https://github.com/craffel/mir_eval for transforming v3 into Python it might indeed be the best place to continue the development there. It seems to be also a good idea to document more explicit on the testing and comparison to the first Matlab implementation as the information is a little hidden in different issues at the moment.

A good argument for its own repository might be that the BSS eval community is maybe not too involved in music information retrieval and the other way around, which means it could be a challenging task to maintain https://github.com/craffel/mir_eval as a whole for a long time (e.g. if the interest in the MIR community for it is lost)

@aliutkus
Copy link
Member

  • we actually also did thorough comparison of v3 with matlab.
  • this v4 implements some new features that make computations way faster (time invariant filters)

I personnaly don't think that having bsseval burried in mireval is much better than having it burried in museval. So yes, I also think a separate repository is the best.

@faroit
Copy link
Member

faroit commented May 18, 2019

we got more feedback for museval at this years ICASSP. Basically people addressed the need for a separate bsseval package with fewer requirements and focus on performance. Therefore I would now propose to split this project and move the metrics to a new bsseval package. Besides bsseval v3 and v4 we could also think of adding SI-SDR and lightweight implementations of SDR in torch or tensorflow suitable for backprop.

@pseeth @Jonathan-LeRoux would love to also hear your feedback of this

@aliutkus
Copy link
Member

sure, it makes total sense, I fully agree on this

@pseeth
Copy link

pseeth commented May 18, 2019

Yeah, happy to contribute. @ethman might have thoughts too. One suggestion is to make any torch/tensorflow dependency optional as those packages can be quite big (won’t fit on an AWS lambda).

@faroit
Copy link
Member

faroit commented May 18, 2019

One suggestion is to make any torch/tensorflow dependency optional as those packages can be quite big (won’t fit on an AWS lambda).

sure. Lets make separate packages bsseval (==numpy), bsseval-numba, bsseval-pytorch ...

I will start with the basic numpy version and then we could go from there

@Jonathan-LeRoux
Copy link

Having bsseval as a separate package definitely makes sense. I'm also in favor of including SI-SDR / SI-SNR (same thing, different name). I guess we could also consider including the redefined SI-SIR and SI-SAR, although I'm still not convinced they can be reliably interpreted in the way people have, so we may be making the community a service by not providing them :)

@faroit
Copy link
Member

faroit commented May 19, 2019

I'm also in favor of including SI-SDR / SI-SNR (same thing, different name). I guess we could also consider including the redefined SI-SIR and SI-SAR, although I'm still not convinced they can be reliably interpreted in the way people have, so we may be making the community a service by not providing them :)

👍 sounds great. @pseeth @Jonathan-LeRoux I will invite you to the repo.

@faroit
Copy link
Member

faroit commented May 21, 2019

done -> bsseval

lets continue discussion over there

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

No branches or pull requests

5 participants