Repairing unit tests for source separation #240
Conversation
|
Appears to still fail :( |
|
Dang. I rolled the threshold up until it passed locally. I guess travis is a different story; trying again at 1e-2. |
|
apparently 1e-2 is still too much. Maybe it's worth removing the offending fixtures instead? |
|
It looks like the two remaining failing tests are still being called with atol 1e-12. |
|
Aha -- the problem is actually that some of the failures are not framewise metrics. At this point, I recommend reverting back to strict test thresholds, but dropping examples 00 and 04 from the fixtures. Were these generated specifically to test any particular failure modes, or are they just random clips? |
I think those (failing) scores are quite low which seem to increase the probability of numerical instabilities. However, they are not edge cases where one of the estimated targets is zero or identical to the reference. |
|
@faroit so would we lose any case coverage by dropping these examples? |
|
short answer: no longer answer: 04 is a nice example of one target (bass) having a really bad estimate particular in the first frames. But it's not the only item with bass separation. So it would still be okay to drop it.
it would be interesting to see if increasing the metric window (currently set to |
bmcfee
force-pushed
the
sourcesep-test-fixture-fix
branch
from
e77c407 to
a083db7
Compare
bmcfee
changed the title
|
Okay, tests pass now that we killed the two offending cases. |
|
@craffel if you're okay with dropping these fixtures, this guy's good to go. |
|
Merged, thank you sir. |
|
thanks for taking care of this! |
This PR should resolve #239 by relaxing the pass condition on frame-wise source separation metrics.
Again, this is a result of instability introduced by solve/lstsq having different implementations depending on the underlying linear algebra system (blas/mkl/openblas/etc).