Fix source separation unit tests

[bmcfee]

Tagging @faroit @ecmjohnson

At some point in the last six months, the regression tests for source separation have started failing, specifically on fixtures output00 (Images Frames - Source to Interference and Artifact) and output04 (Images - Source to Interference and Artifact).

Anyone know what's going on here? The results are off by up to 1e-2 in some cases, so I don't think this is up to json encoding round-off error. Maybe something changed under the hood in numpy or scipy since the fixtures were generated? If that's all it is, then it should be a simple matter of revising the expected scores.

It looks like these fixtures were most recently updated by @ecmjohnson in #212 .

[bmcfee]

FWIW, I get slightly different failures on my local machine compared to travis.

Travis failure (2.7):

test_separation.test_separation_functions('data/separation/output00.json', 'Images Frames - Source to Interference',
[[9.758714270276117, 11.253809511512536], [6.2010410615210985, 5.744778149349782], [16.629170662005855, 14.244503227809638]],
[[9.760140854576726, 11.253796559312175], [6.201456318343639, 5.744776282225366], [16.629136350117925, 14.244627662024063]])
... FAIL

Travis failure (3.5):

test_separation.test_separation_functions('data/separation/output00.json', 'Images Frames - Source to Interference',
[[9.758714270276117, 11.253809511512536], [6.2010410615210985, 5.744778149349782], [16.629170662005855, 14.244503227809638]],
[[9.760140854576726, 11.253796559312175], [6.201456318343639, 5.744776282225366], [16.629136350117925, 14.244627662024063]]) 
... FAIL

Local failure (3.5):

test_separation.test_separation_functions('data/separation/output00.json', 'Images Frames - Source to Interference',
[[9.759960511788385, 11.253809724548846], [6.201542302501348, 5.744774076828487], [16.629147059167934, 14.244501112492447]],
[[9.760140854576726, 11.253796559312175], [6.201456318343639, 5.744776282225366], [16.629136350117925, 14.244627662024063]])
... FAIL

My local environment is pretty close to the travis 3.5 environment, except that I'm using an openblas numpy build.

Looking into the code, I wonder if there are machine precision errors creeping in through a numerically unstable db calculation here. This would be better done as:

EDIT

if den < 1e-50:
    return np.Inf
return 10 * (np.log10(num) - np.log10(den))

[bmcfee]

The plot thickens. On a local environment using the stock conda numpy build, it passes:

test_separation.test_separation_functions('data/separation/output00.json', 'Images Frames - Source to Interference',
[[9.760127118373102, 11.253809794172023], [6.201405192648897, 5.7447738549088045], [16.629167773208714, 14.24450255084902]],
[[9.760140854576726, 11.253796559312175], [6.201456318343639, 5.744776282225366], [16.629136350117925, 14.244627662024063]])
... ok

The fact that swapping out the blas implementation can trigger failures in these tests indicates that there's indeed some numerically unstable calculation lurking somewhere in the separation module. I'm not sure it's the safe_db calculation, as fixing that locally but keeping openblas did not fix it on its own.

[bmcfee]

After a bit more digging, I think the heisen/eigen-bug can be tracked down to random and unpredictable failures in linalg here, where it first tries to do an exact linalg.solve and then falls back on linalg.lstsq. I think, in some cases, certain of our test examples can be solved with some blas backends but not others, leading to slight numerical differences in the outcomes.

What do people think about always using lstsq, and ideally, putting an explicit cutoff for the condition number (rcond) parameter so that the results are stable across backends?

[bmcfee]

What do people think about always using lstsq, and ideally, putting an explicit cutoff for the condition number (rcond) parameter so that the results are stable across backends?

Gave this a spin, it's horrendously slow.

Update after probing at this quite a bit, I think the only reasonable solution is to either A) drop these test cases, or B) relax the tolerance for passing the tests.

I personally would opt for (B), since the errors are relatively small anyway, and since we're in log space (dB) we can afford to tolerate more slop than we would in linear space.

Actually fixing this would require significantly changing the numerical behavior of the frame-based evaluation, which isn't something I think any of us are keen on doing.

[faroit]

B) 😢 we spent so long to get that close to matlabs output... but yes, I think this is okay.

I guess dropping blas-enabled numpy builds for the unit tests is not an option for you anaconda users, right?

[faroit]

@bmcfee did you test with stock numpy vs. mkl or did you try even more blas backends?

[bmcfee]

I guess dropping blas-enabled numpy builds for the unit tests is not an option for you anaconda users, right?

They're all blas enabled, it's just a question of which blas. We could try to do a better job of matching the outputs to the test environments, but I think that's sort of missing the point, and wouldn't ensure that different users would get the same results on the same input anyway.

did you test with stock numpy vs. mkl or did you try even more blas backends?

I tried locally on these two numpy builds.

This one passes:

In [6]: np.__config__.show()
lapack_opt_info:
    library_dirs = ['/home/bmcfee/miniconda/envs/clean/lib']
    define_macros = [('SCIPY_MKL_H', None), ('HAVE_CBLAS', None)]
    include_dirs = ['/home/bmcfee/miniconda/envs/clean/include']
    libraries = ['mkl_intel_lp64', 'mkl_intel_thread', 'mkl_core', 'iomp5', 'pthread']
blas_opt_info:
    library_dirs = ['/home/bmcfee/miniconda/envs/clean/lib']
    define_macros = [('SCIPY_MKL_H', None), ('HAVE_CBLAS', None)]
    include_dirs = ['/home/bmcfee/miniconda/envs/clean/include']
    libraries = ['mkl_intel_lp64', 'mkl_intel_thread', 'mkl_core', 'iomp5', 'pthread']
lapack_mkl_info:
    library_dirs = ['/home/bmcfee/miniconda/envs/clean/lib']
    define_macros = [('SCIPY_MKL_H', None), ('HAVE_CBLAS', None)]
    include_dirs = ['/home/bmcfee/miniconda/envs/clean/include']
    libraries = ['mkl_intel_lp64', 'mkl_intel_thread', 'mkl_core', 'iomp5', 'pthread']
openblas_lapack_info:
  NOT AVAILABLE
blas_mkl_info:
    library_dirs = ['/home/bmcfee/miniconda/envs/clean/lib']
    define_macros = [('SCIPY_MKL_H', None), ('HAVE_CBLAS', None)]
    include_dirs = ['/home/bmcfee/miniconda/envs/clean/include']
    libraries = ['mkl_intel_lp64', 'mkl_intel_thread', 'mkl_core', 'iomp5', 'pthread']

This one fails:

In [2]: np.__config__.show()
lapack_mkl_info:
  NOT AVAILABLE
openblas_info:
    language = c
    library_dirs = ['/home/bmcfee/miniconda/envs/py35/lib']
    define_macros = [('HAVE_CBLAS', None)]
    libraries = ['openblas', 'openblas']
blas_opt_info:
    language = c
    library_dirs = ['/home/bmcfee/miniconda/envs/py35/lib']
    define_macros = [('HAVE_CBLAS', None)]
    libraries = ['openblas', 'openblas']
blis_info:
  NOT AVAILABLE
blas_mkl_info:
  NOT AVAILABLE
openblas_lapack_info:
    language = c
    library_dirs = ['/home/bmcfee/miniconda/envs/py35/lib']
    define_macros = [('HAVE_CBLAS', None)]
    libraries = ['openblas', 'openblas']
lapack_opt_info:
    language = c
    library_dirs = ['/home/bmcfee/miniconda/envs/py35/lib']
    define_macros = [('HAVE_CBLAS', None)]
    libraries = ['openblas', 'openblas']

[craffel]

B) relax the tolerance for passing the tests.

This seems fine to me.