Division by zero causes a test failure in unifrac

[vpa1977]

=====================================================================
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 718, in test_faith_pd_none_observed
    self.assertAlmostEqual(actual[0], expected)
AssertionError: 6.25 != 0.0 within 7 places (6.25 difference)

This is due void su::set_proportions() dividing props[i] by sample_count[i] which is 0.

In order to avoid undefined behaviour the value of props[i] can by adjusted to std::numeric_limits::max()/lowest()

[wasade]

Does this exist with the latest version?

…

On Wed, Oct 4, 2023, 13:24 Vladimir Petko ***@***.***> wrote: ===================================================================== FAIL: test_faith_pd_none_observed (unifrac.tests.test_api.FaithPDEdgeCasesTests.test_faith_pd_none_observed) Traceback (most recent call last): File "/<>/.pybuild/cpython3_3.11_unifrac/build/unifrac/tests/test_api.py", line 718, in test_faith_pd_none_observed self.assertAlmostEqual(actual[0], expected) AssertionError: 6.25 != 0.0 within 7 places (6.25 difference) This is due void su::set_proportions() dividing props[i] by sample_count[i] which is 0. In order to avoid undefined behaviour the value of props[i] can by adjusted to std::numeric_limits::max()/lowest() — Reply to this email directly, view it on GitHub <#51>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADTZMS7QJKGVFCAJ2UOJNDX5XAWZAVCNFSM6AAAAAA5TF7322VHI2DSMVQWIX3LMV43ASLTON2WKOZRHEZDMOJSGM2TOMY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[vpa1977]

Yes, it does.

$ python3 unifrac/tests/test_api.py 
...........................F............
======================================================================
FAIL: test_faith_pd_none_observed (__main__.FaithPDEdgeCasesTests.test_faith_pd_none_observed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vladimirp/git/plusone/github/unifrac/unifrac/tests/test_api.py", line 717, in test_faith_pd_none_observed
    self.assertAlmostEqual(actual[0], expected)
AssertionError: 6.25 != 0.0 within 7 places (6.25 difference)

----------------------------------------------------------------------
Ran 40 tests in 3.401s

FAILED (failures=1)

Note: I have patched the test due to biocore/unifrac#156

   def test_faith_pd_none_observed(self):
        actual = self.faith_pd_work([0, 0, 0, 0, 0], self.oids1, ['foo'],
                                    self.t1)
        expected = 0.0
        self.assertAlmostEqual(actual[0], expected)

[wasade]

Thats puzzling as the suite is executed by CI. Will look into it, thanks!

…

On Wed, Oct 4, 2023, 17:28 Vladimir Petko ***@***.***> wrote: yes it does — Reply to this email directly, view it on GitHub <#51 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADTZMVSM27KAXQKVIBW55TX5X5JJAVCNFSM6AAAAAA5TF7322VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBXHA2DCNRVG4> . You are receiving this because you commented.Message ID: ***@***.***>

[vpa1977]

I think it is due to some dependency - in void biom_inmem::compute_sample_counts() n_samples is 1, but resident_obj.obs_counts_resident[i]; is 0, which leaves the sample_count[0] as 0.

[sfiligoi]

@vpa1977 I need a way to reproduce this.
I just re-installed the latest unifrac (as per CI), and it passes all tests with flying colors.

The most likely explanation is that something in your setup is picking up something it should not.
But I cannot help unless I can reproduce.

(unifrac-dev-311) -bash-4.2$ python3 unifrac/tests/test_api.py 
/panfs/isfiligoi/t8/t1/t2/unifrac/unifrac/tests/test_api.py:12: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
............................/panfs/isfiligoi/t8/t1/t2/unifrac/unifrac/tests/test_api.py:814: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
  self.assertAlmostEqual(actual[0], expected)
/panfs/isfiligoi/t8/t1/t2/unifrac/unifrac/tests/test_api.py:820: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
  self.assertAlmostEqual(actual[0], expected)
.../panfs/isfiligoi/t8/t1/t2/unifrac/unifrac/tests/test_api.py:711: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
  self.assertAlmostEqual(actual[0], expected)
.........
----------------------------------------------------------------------
Ran 40 tests in 4.871s

OK

[vpa1977]

Yes, it is a bit flaky, due to the UB, so it might pass in some cases[1].

[1] #52 (comment)