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

Tests fail on Ubuntu 16.04 and above #78

Closed
MatzFan opened this issue May 12, 2019 · 6 comments
Closed

Tests fail on Ubuntu 16.04 and above #78

MatzFan opened this issue May 12, 2019 · 6 comments

Comments

@MatzFan
Copy link
Collaborator

MatzFan commented May 12, 2019

I installed from master, complied and ran the tests on my local system (Ubuntu Bionic 18.04). The following is the relevant part of the result:

  1) Failure:
TestPhashion#test_mh_distance_from [/home/me/dev/phashion/test/test_phashion.rb:90]:
Expected |0.1 - 0.06770833333333333| (0.03229166666666668) to be <= 0.028.

  2) Failure:
TestPhashion#test_mh_hash_for [/home/me/dev/phashion/test/test_phashion.rb:83]:
Expected |0.1 - 0.06770833333333333| (0.03229166666666668) to be <= 0.028.

21 runs, 69 assertions, 2 failures, 0 errors, 0 skips

My fork using Travis-CI Xenial (16.04) produces the same result, as you can see here. I have not checked whether whatever causes this affects the fingerprint calculation as well as mh_hash.

Library versions (my local machine):
libpng-dev 1.6.34
libjpeg-dev 8c

konstantinoskostis added a commit to skroutz/phashion that referenced this issue Jun 29, 2020
This commit adjusts the thresholds in two tests regarding the mexican
hat calculation functionality. It increases the thressholds from the
value of `0.028` to `0.033`.
The same two tests are failing for Debian Stretch with ruby 2.3.3
and for Debian Buster with ruby 2.5.5. There is also a github issue
regarding these same tests for Ubuntu 16.04[1]
It seems that this issue is a floating point issue but somewhat
related to the OS, because linux trusty specs[2] pass whereas the specs for
linux xenial[3] do not and the code has not changed.
See also here[4], where the thresholds for these same tests have
been modified by the original authors without justifying the change.

[1]: westonplatter#78
[2]: https://travis-ci.org/github/MatzFan/phashion/jobs/531359477
[3]: https://travis-ci.org/github/MatzFan/phashion/jobs/531367883
[4]: b6f3a77
@westonplatter
Copy link
Owner

@MatzFan thanks for seeing this and making the adjustments. This looks like the right move, especially since there have been no changes to the ruby or underlying C library. Merging :)

@westonplatter
Copy link
Owner

@MatzFan thanks for demonstrating the resolve code changes. I copied your same approach in #88.

Would you have any interest in helping maintain phashion and release new versions? I don't spend much time in the ruby world anymore so digging in the details here is harder for me. If you're interested, let's talk.

@MatzFan
Copy link
Collaborator Author

MatzFan commented Jun 8, 2022

@westonplatter happy to give it a go, so long as I don't need to go near the C library! Not used phashion since my attempt to crack Google's reCAPTCHA a few years ago. I still have millions of pHashes of street signs somewhere..

@westonplatter
Copy link
Owner

@MatzFan added you as a repo collaborator.

@westonplatter
Copy link
Owner

Given the CI is running successfully on ubunut-22.04, I think I can close this out. @MatzFan does closing this issue make sense to you?

@MatzFan
Copy link
Collaborator Author

MatzFan commented Mar 6, 2023

@westonplatter agreed.

@MatzFan MatzFan closed this as completed Mar 6, 2023
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

2 participants