[CPP] Remove zero diameter check for edges in 0-dim computation

[ulupo]

Fixes #31, adding a unit test for "equivariance" (the property I was checking for in the example in #31) which fails in main but not after the fix.

New version of #41.

[ulupo]

@MonkeyBreaker the test is good, but unlike what I confidently stated above it does not fail (or fails so rarely I couldn't make it fail) on current main. Evidently hypothesis does not produce something like my example in the discussion in #31 (for which I had to pick a specific random seed).

What I am thinking is:

• keep the current test as is -- we like hypothesis
• add my specific example by hand at the end of the test
  • make sure once again that this addition does fail in main

[MonkeyBreaker]

keep the current test as is -- we like hypothesis

Well we must be confident that our test fails if we break something.
I am not a fan of having test that won't fail at each time that we run if we broke the corresponding feature tested.

add my specific example by hand at the end of the test

Well, instead of having at the end of the test, implement 2 test, one with hypothesis that produces multiple tests.
One with the hard-coded input data that we know fails if the fix is not implemented.
I the input matrix is small, I would hard-code the matrix directly, because if we depend on the behavior of np.seed() it is not IMO reliable enough.

make sure once again that this addition does fail in main

Sure, I can also cross-validate

[ulupo]

Well we must be confident that our test fails if we break something.
I am not a fan of having test that won't fail at each time that we run if we broke the corresponding feature tested.

Not sure I follow -- this would definitely happen after my edits. But it is the same test -- for the same property. Testing with hypothesis covers tricky things like overflows, etc, while the hard-coded example makes sure we don't regress. But it is the same property (equivariance) that is being tested.

[ulupo]

On the other hand, I'm happy to split the tests -- e.g. because otherwise the same specific dm will be tested multiple times, one per run made by hypothesis. This alone is a good reason for me to split!

[ulupo]

@MonkeyBreaker I have now added the separate explicit regression test as discussed. I checked that it fails on main currently.

Let me know if you'd like further changes for this PR.

[MonkeyBreaker]

Not sure I follow -- this would definitely happen after my edits. But it is the same test -- for the same property. Testing with hypothesis covers tricky things like overflows, etc, while the hard-coded example makes sure we don't regress. But it is the same property (equivariance) that is being tested.

I am good that we have 2 separate test for validating a feature. I was afraid that with only 1 test (without the case we are always sure it fails), we could not really with certitude if all test are good.

But now, with both of the test implemented, I am happy :)

[MonkeyBreaker]

LGTM !
I could reproduce the issue on my computer with your test and after adding the fix, all test passes !