search

mariomulansky / PySpike

Python implementation of spike distance metrics
http://mariomulansky.github.io/PySpike
Other
71 stars 30 forks source link

Tests should not test for exact equality (tests fail on certain architectures) #44

Closed gspr closed 2 years ago

gspr commented 4 years ago

My Debian package of PySpike has just been included into the distribution [1]. The build process revealed that some of the included tests fail on certain architectures. I believe you should not be using assert_equal in these tests, but rather assert_allclose and friends to account for slightly different floating point behavior on different architectures.

[1] https://tracker.debian.org/pkg/python-pyspike

mariomulansky commented 4 years ago

Firstly, thanks for pushing PySpike into a Debian package!

I only have CI testing on Linux for PySpike so other platforms might have issues indeed. Do you have a build log of the failures somewhere? I followed you link but I couldn't find any. I'd be happy to fix the errors, but it would be very helpful if I had a way to reproduce the failure.

Thanks

gspr commented 4 years ago

Here's a log example. Just search for FAIL to find the relevant portion.

I am about to test a fix, and will report back.

gspr commented 4 years ago

This patch seems sufficient. I tested it on one of the affected architectures (arm64).

Let me know if you prefer a pull request. The patch essentially just replaces all occurrences of assert_equal with assert_allclose.

mariomulansky commented 4 years ago

Thanks! If it's not too much hassle I would appreciate a PR - please open it against the develop branch. I'll make sure to push a new release from that. I appreciate your efforts here!

PS: PySpike running on ARM64 - that's pretty awesome :)

gspr commented 4 years ago

Mario Mulansky notifications@github.com writes:

Thanks! If it's not too much hassle I would appreciate a PR - please open it against the develop branch. I'll make sure to push a new release from that. I appreciate your efforts here!

I try to avoid using GitHub as much as I can, so here's a non-GitHub pull request:

The following changes since commit 8ab39a445781175d1c8732d24ff30e90f8d479ab:

Add missing line in tutorial (2018-09-20 10:58:13 -0700)

are available in the Git repository at:

https://git.nonempty.org/debian-python-pyspike/ gspr/issue-44

for you to fetch changes up to 9649685210a8b5252b28d8289d2c8258c28ed24b:

Use assert_allclose instead of assert_equal in tests to allow for different floating point behavior on different architectures or optimization levels. (2020-03-08 11:29:48 +0100)

Gard Spreemann (1): Use assert_allclose instead of assert_equal in tests to allow for different floating point behavior on different architectures or optimization levels.

test/test_distance.py | 70 +++++++++++++++--------------- test/test_empty.py | 36 +++++++-------- test/test_function.py | 52 +++++++++++----------- test/test_generic_interfaces.py | 18 ++++---- test/test_regression/test_regression_15.py | 20 ++++----- test/test_spikes.py | 10 ++--- test/test_sync_filter.py | 32 +++++++------- 7 files changed, 119 insertions(+), 119 deletions(-)

PS: PySpike running on ARM64 - that's pretty awesome :)

Hehe, and a bunch of other architectures too:

https://buildd.debian.org/status/package.php?p=python-pyspike

How many users there are on those architectures I don't know.

Migration of the package into Ubuntu and other Debian-derived distributions should happen within a few days too.

gspr commented 3 years ago

A gentle ping on this issue, if you don't mind :-)

gspr commented 3 years ago

I've also pushed the PR within GitHub's system, if that eases merging.