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

Loosen separation test tolerance #302

Merged
merged 2 commits into from Jan 11, 2019
Merged

Conversation

craffel
Copy link
Owner

@craffel craffel commented Jan 9, 2019

For some tests, the difference between the scores produced by Travis' build and my own build is as large as 3.89588121e-04. This was causing Travis to fail. In practice, we shouldn't care much about differences larger than 10e-3 for separation tasks since it would be very unusual for anyone to pay attention to differences this small when comparing source separation algorithms.

For some tests, the difference between the scores produced by Travis'
build and my own build is as large as 3.89588121e-04. In practice, we
shouldn't care much about differences larger than 10e-3 for separation
tasks since it would be very unusual for anyone to pay attention to
differences this small when comparing source separation algorithms.
@bmcfee
Copy link
Collaborator

bmcfee commented Jan 9, 2019

LGTM; I think this is related to our previous headaches deriving from BLAS discrepancies across platforms, but I agree that anything past the 3rd (2nd?) decimal place is unreliable in source sep anyway.

@craffel
Copy link
Owner Author

craffel commented Jan 9, 2019

Amazingly, this is still not loose enough for the Python 3.4 build:
https://travis-ci.org/craffel/mir_eval/jobs/477467832
The max absolute deviation between my system and Travis' system is 4.87191262e-03. What do we think about this? @faroit

@faroit
Copy link

faroit commented Jan 9, 2019

  • Do we still need to support 3.4?
  • I totally agree to loosen it further. Bsseval already caused way too much trouble over here ;-)

@craffel
Copy link
Owner Author

craffel commented Jan 10, 2019

Do we still need to support 3.4?

Not necessarily, but we have just not updated Travis. That this affects 3.4 and not, say, 3.5 or 3.6 is mostly a coincidence I think.

I totally agree to loosen it further. Bsseval already caused way too much trouble over here ;-)

Ok, will do.

@bmcfee
Copy link
Collaborator

bmcfee commented Jan 10, 2019

Not necessarily, but we have just not updated Travis. That this affects 3.4 and not, say, 3.5 or 3.6 is mostly a coincidence I think.

I dropped 3.4 builds on travis (in librosa) for exactly this reason. Since mir_eval doesn't use any fancy features of >=3.5 (except, maybe, implicitly ordered dictionaries?), I don't see much point in keeping the 3.4 test around.

@craffel craffel merged commit 6e9f59b into master Jan 11, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants