Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor handling of coincident points in triangulation #714

Merged
merged 18 commits into from
May 30, 2024

Conversation

martinfleis
Copy link
Member

Closes #713

Had to touch more stuff than intended but we now rely on QHull to identify coincident points and provide a lookup from which we can build cliques. This should ensure that we won't hit the precision issues.

Some minor cleanup happened alongside, i.e. to avoid solving isolates twice in a row.

Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.9%. Comparing base (bcabdbc) to head (4d7eb44).
Report is 16 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #714     +/-   ##
=======================================
- Coverage   85.0%   84.9%   -0.1%     
=======================================
  Files        141     145      +4     
  Lines      15203   15393    +190     
=======================================
+ Hits       12924   13075    +151     
- Misses      2279    2318     +39     
Files Coverage Δ
libpysal/graph/_kernel.py 82.0% <100.0%> (-0.1%) ⬇️
libpysal/graph/_triangulation.py 98.0% <100.0%> (+0.1%) ⬆️
libpysal/graph/_utils.py 96.3% <100.0%> (+1.4%) ⬆️
libpysal/graph/base.py 97.0% <100.0%> (-1.0%) ⬇️
libpysal/graph/tests/test_builders.py 100.0% <100.0%> (ø)
libpysal/graph/tests/test_kernel.py 99.0% <100.0%> (+<0.1%) ⬆️
libpysal/graph/tests/test_triangulation.py 98.8% <100.0%> (+<0.1%) ⬆️

... and 12 files with indirect coverage changes

libpysal/graph/_triangulation.py Outdated Show resolved Hide resolved
libpysal/graph/_triangulation.py Show resolved Hide resolved
@martinfleis martinfleis marked this pull request as draft May 30, 2024 09:00
@martinfleis martinfleis marked this pull request as ready for review May 30, 2024 10:23
libpysal/graph/_kernel.py Show resolved Hide resolved
libpysal/graph/_kernel.py Show resolved Hide resolved
@martinfleis
Copy link
Member Author

Okay, this is now significantly updated.

  • switch terminology from coincident to coplanar as suggested by @ljwolf (cc @knaaptime in case you are using "coincident" somewhere in your materials, this is technically a breaking change but we're still in experimental mode)
  • knn uses the same coplanar resolver as triangulation. that also required new function to build coplanar lookup to match the one coming from Delaunay
  • raising CoplanarError
  • always jitter when coplanar="jitter", no matter if there is a coplanar point or not

The performance is about the same as it was before.

@martinfleis martinfleis merged commit 6dba8b6 into pysal:main May 30, 2024
11 checks passed
@martinfleis martinfleis deleted the triangulation-coincident branch May 30, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Co-location issues with Graph triangulation builders
3 participants