deprecating separation module

[bmcfee]

This PR deprecates the separation module.

Question for @faroit - is sigsep-museval the best replacement to recommend here?

Question for @craffel - should we disable tests on the separation module? They've always been finnicky and the vast majority of build time. If we're deprecating the module anyway, I could see some benefit to disabling these tests.

[craffel]

Thanks. Yes, let's disable the tests too.

[faroit]

@bmcfee thanks for the initiative. Before we merge this, I would guess we should get some feedback from the community. museval might be a good fit as its an implementation of the bsseval images (unlike _sources) algorithm but it might needs some work to make it future proof as well: wrt to the precision issue, I also had to lower the regression tests tolerance https://github.com/sigsep/sigsep-mus-eval/pull/93/files#diff-6edc05e4134fe68983d08319a22f62d5e4efdd829656595282cf6ef97011c13aL90 myself (testing against older version of museval with older numpy version).

Here are some people that I would def value their opinion on how to proceed with the sdr metrics:

@rabitt @fakufaku @iver56 @adefossez

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.47%. Comparing base (fddf120) to head (64db1ed).

Files	Patch %	Lines
mir_eval/util.py	71.42%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #382       +/-   ##
===========================================
- Coverage   95.73%   85.47%   -10.27%     
===========================================
  Files          19       19               
  Lines        2934     2946       +12     
===========================================
- Hits         2809     2518      -291     
- Misses        125      428      +303     

Flag	Coverage Δ
unittests	85.47% <83.33%> (-10.27%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bmcfee]

Thanks. Yes, let's disable the tests too.

Done.

museval might be a good fit as its an implementation of the bsseval images (unlike _sources) algorithm but it might needs some work to make it future proof as well: wrt to the precision issue, I also had to lower the regression tests tolerance https://github.com/sigsep/sigsep-mus-eval/pull/93/files#diff-6edc05e4134fe68983d08319a22f62d5e4efdd829656595282cf6ef97011c13aL90 myself

I think the precision issue will always be there as long as we're relying on unregularized least squares; even moreso if we're dynamically switching between solve() and lstsq(), as mir_eval and museval both do. This dynamic behavior, IIRC, was intended to mimic what matlab's \ operator does under the hood, and it's always seemed troublesome to me. Museval's implementation at least puts a ridge on the solve(), so it really shouldn't ever hit the lstsq path, but as I noted in the other PR thread #381 , the exception handling there isn't quite correct anymore.

My not-so-humble opinion is that all of these implementations should probably be scrapped and rewritten 😅 But if we're going to do that (as a community), it should only happen once, and in a dedicated package.

[bmcfee]

Side note: failures on 3.12 builds are due to matplotlib removeing BrokenBarHCollection. Will fix that in a separate PR.

[craffel]

Is there a way to tell codecov not to worry about the separation file's lines so it hates us less?

[fakufaku]

Jumping in the discussion here. I have written a fast bss_eval_sources package (fast_bss_eval). At the time, the fast routine was quite faster than the sigsep/bsseval implementation. It also supports numpy/torch, fp32/fp64, cpu/gpu, and solve or conjugate GD.

I think the implementation ideas are applicable to the _images routine for which the savings are potentially even larger.

edit: paper link

[iver56]

I'm okay with this. I don't calculate SIR, SAR and ISR anymore when evaluating source separation models/algorithms. I still calculate SDR (with a stripped fork of sigsep/bsseval), but I mostly don't use it. I mainly rely on logWMSE in combination with a few others.

[bmcfee]

Is there a way to tell codecov not to worry about the separation file's lines so it hates us less?

I've added separation to the omit section, but it might take a merge before codecov stops comparing to old results that did include it.

[bmcfee]

Side note: failures on 3.12 builds are due to matplotlib removing BrokenBarHCollection. Will fix that in a separate PR.

Actually just rolled it into this one - all that needed to happen was to remove an import tweak a docstring, no actual code changes.

[bmcfee]

Ok, I think this is good to merge now. All matplotlib issues are resolved and tests are workign out of the box. @craffel ?

[bmcfee]

Yerp, realized this was already approved.