support cliques as a resolver of coincident points in Graph

[knaaptime]

i think we were just missing a couple parens

[martinfleis]

The issue, as far as I remember it, was that _induce_cliques was not working properly. That is why I added NotImplementedError. I'd be happy to allow it again but the PR needs to come with extensive tests to ensure it all works as it is supposed to :).

[ljwolf]

I think there was an issue after a bad rebase.

I can get some attention on it this week to fix.

[ljwolf]

I'm pretty certain that the issue was induced when we switched from focal-indexed adjacency to non-indexed adjacency.. @knaaptime, I've sent a PR to your clique branch with the change.

The dual merge in lines 76-82 was supposed to match up the deduped ids back onto the clique ids, and then explode the cliques into their own links. This only works when the adjtable is indexed on focal.

With this fix, tests still fail, but I think the test answers are incorrect? In the tests, 0 and 4 are coincident, and the current answers are:

exp_heads = np.array(
    [0, 0, 0, 1, 1, 1, 1, 1, 2, 2, 2, 3, 3, 3, 4, 4, 4, 5, 5, 5]
)
exp_tails = np.array(
    [1, 2, 4, 0, 2, 3, 4, 5, 0, 1, 3, 1, 2, 5, 0, 1, 5, 1, 3, 4]
)

But, if 0 and 4 are coincident, then every link and backlink containing 0 should also contain 4. So, for example, link (5,0) is missing, as is link (2,4). So, the correct answer, I think, is:

exp_heads = numpy.array([0, 0, 0, 0, 1, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 4, 4, 4, 4, 5, 5,
       5, 5])
exp_tails = numpy.array([1, 2, 4, 5, 0, 2, 3, 4, 5, 0, 1, 3, 4, 1, 2, 5, 0, 1, 2, 5, 0, 1,
       3, 4])

The _induce_cliques() function with f9bfadbe correctly recovers this.

[ljwolf]

OK, after some further testing, I think I've confirmed the answers need to change, but I had to make sure to re-set the working values (like, coordinates, ids, geoms, heads, tails, weights) after expanding the cliques.

I will change the tests, but the correct edge table is in the comment above.

[ljwolf]

This now also addresses #573. The fix was as described, but also exposed a few other minor issues in the implementation that have been fixed.

[ljwolf]

Triangulation is done. I'm now verifying the usage in _knn()/_kernel(), so still WIP.

[ljwolf]

_knn/_kernel is done. Currently, I'm getting local failures in the round-tripping from sparse? The adjacency tables are the same, but comparing the graphs by equality is not. I'm not sure how/where this would have been introduced by this PR, and it's currently failing on main?

[ljwolf]

This is ready for review.

[martinfleis]

Currently, I'm getting local failures in the round-tripping from sparse? The adjacency tables are the same, but comparing the graphs by equality is not. I'm not sure how/where this would have been introduced by this PR, and it's currently failing on main?

Could it be some issue with your local env? It seems to be work as expected on CI.

We have some issue with coincident points in the env with the minimum supported versions of dependencies for some reason...

[ljwolf]

then it must be something in my local environment. I will check out the failures in py3.10. I think that's likely a numba issue.

[codecov]

Codecov Report

Merging #647 (ab9eef1) into main (44204b5) will increase coverage by 0.6%.
Report is 46 commits behind head on main.
The diff coverage is 98.1%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #647     +/-   ##
=======================================
+ Coverage   84.5%   85.0%   +0.6%     
=======================================
  Files        139     139             
  Lines      14972   14868    -104     
=======================================
- Hits       12647   12642      -5     
+ Misses      2325    2226     -99     

Files	Coverage Δ
libpysal/graph/_kernel.py	88.8% <100.0%> (+0.7%)	⬆️
libpysal/graph/_utils.py	94.1% <100.0%> (+4.8%)	⬆️
libpysal/graph/tests/test_builders.py	100.0% <100.0%> (ø)
libpysal/graph/tests/test_kernel.py	100.0% <100.0%> (ø)
libpysal/graph/tests/test_triangulation.py	98.8% <100.0%> (+14.5%)	⬆️
libpysal/graph/tests/test_utils.py	98.8% <100.0%> (ø)
libpysal/graph/_triangulation.py	97.9% <94.1%> (+0.7%)	⬆️

... and 51 files with indirect coverage changes

[ljwolf]

Not a numba issue, but rather needed a bit more than 4091467 to fully fix #573.

[ljwolf]

Tests are passing, ready for review!

[ljwolf]

@knaaptime I can't assign you as a reviewer, but you probably should be one of the reviewers anyway, having motivated me to revisit this 😄

[knaaptime]

you rule. But i also cant approve, apparently, since it's technically "my" pr

so, approved. :P

[martinfleis]

I can approve :)