Test failures on unifrac 1.3 release

[nileshpatra]

Hi, Tests fail on this release with:

======================================================================
FAIL: test_unweighted_minimal_trees (unifrac.tests.test_api.EdgeCasesTests.test_unweighted_minimal_trees)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/<<PKGBUILDDIR>>/.pybuild/cpython3_3.11_unifrac/build/unifrac/tests/test_api.py", line 296, in test_unweighted_minimal_trees
    self.assertEqual(actual, expected)
AssertionError: 0.9999999403953552 != 1.0

======================================================================
FAIL: test_faith_pd_none_observed (unifrac.tests.test_api.FaithPDEdgeCasesTests.test_faith_pd_none_observed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/<<PKGBUILDDIR>>/.pybuild/cpython3_3.11_unifrac/build/unifrac/tests/test_api.py", line 717, in test_faith_pd_none_observed
    np.testing.assert_array_almost_equal(actual.values, expected, decimal=4)
  File "/usr/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/numpy/testing/_private/utils.py", line 1099, in assert_array_almost_equal
    assert_array_compare(compare, x, y, err_msg=err_msg, verbose=verbose,
  File "/usr/lib/python3.11/contextlib.py", line 81, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/numpy/testing/_private/utils.py", line 862, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not almost equal to 4 decimals

Mismatched elements: 1 / 1 (100%)
Max absolute difference: 6.25
Max relative difference: inf
 x: array([6.25])
 y: array(0.)

----------------------------------------------------------------------
Ran 49 tests in 6.438s

FAILED (failures=2)

The first one looks like it could be fixed with increasing the tolerance but not sure about the failure in test_faith_pd_none_observed

/cc: @wasade

[sfiligoi]

Hi @nileshpatra . What platform is this on?
This looks very similar to #157

[nileshpatra]

This is happening with unifrac's debian package. I tried with 1.4 version of unifrac-binaries as well, but I still get the same error.

[sfiligoi]

Is this on x86 or some other architecture?
The faith_pd problem was fixed in the unifrac-binaries backend library 51, but has not yet been released.

[nileshpatra]

Happens for me on x86. I confirm that the faith_pd issue goes away with the patch you linked to - I think I'll apply that patch to the debian package.

However the issue with test_unweighted_minimal_trees still persists. I guess it'd be OK to increase the tolerance for that test a little bit since it is a frequent occurance. WDYT?

[nileshpatra]

Taking a deeper look at the issue/PR you linked to and @vpa1977 comments on it- #156 (comment)

When I actually downloaded the debian tarball, I saw this patch applied, so I'm not sure if this is actually tested without the patch? @vpa1977 could you confirm the package actually build with you without this?

$ cat /tmp/round_test_result.patch
Description: Round test result
 Relax test assertion for the floating number comparison. 
 This issue is not reproducible with the latest upstream unifrac-tools.
Author: Vladimir Petko <[email protected]>
Bug: https://github.com/biocore/unifrac/issues/157
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/unifrac-tools/+bug/2038397
Forwarded: not-needed
Last-Update: 2023-10-05
--- a/unifrac/tests/test_api.py
+++ b/unifrac/tests/test_api.py
@@ -293,7 +293,7 @@
         actual = self.unweighted_unifrac([1, 0], [0, 0], ['OTU1', 'OTU2'],
                                          tree)
         expected = 1.0
-        self.assertEqual(actual, expected)
+        self.assertAlmostEqual(actual, expected, places=6)
 
     def test_unweighted_root_not_observed(self):
         # expected values computed with QIIME 1.9.1 and by hand

[sfiligoi]

I think using almost equal is fine.
It is fp compute.

[vpa1977]

I've tested it with the patch. I believe it was needed for armhf.