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

Faster node density #609

Merged
merged 7 commits into from
Jun 17, 2024
Merged

Faster node density #609

merged 7 commits into from
Jun 17, 2024

Conversation

u3ks
Copy link
Collaborator

@u3ks u3ks commented Jun 17, 2024

I moved the node density functionality to the graph module and use networkx to compute it.
The current implementation scales horribly and on larger graphs takes hours, since all edges have to be filtered multiple times with np.in1d.
Getting the exact timing is a bit messy since subgraphs and higher order pysal have to be precomuted and affect the time.
The new implementation takes around <20ish mintues on a graph with ~500k nodes and 2mil edges, together with the other mm.subgraph computations.
For smaller datasets the current implementation + calculating a higher order graph ~ the new networkx implementation.

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.0%. Comparing base (4037c70) to head (9ed20b3).
Report is 47 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #609     +/-   ##
=======================================
+ Coverage   97.4%   98.0%   +0.6%     
=======================================
  Files         26      37     +11     
  Lines       4328    6079   +1751     
=======================================
+ Hits        4214    5957   +1743     
- Misses       114     122      +8     
Files Coverage Δ
momepy/functional/_intensity.py 100.0% <100.0%> (ø)
momepy/functional/tests/test_intensity.py 100.0% <100.0%> (ø)
momepy/graph.py 98.9% <100.0%> (+0.1%) ⬆️

Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

momepy/graph.py Outdated Show resolved Hide resolved
momepy/graph.py Show resolved Hide resolved
momepy/graph.py Outdated Show resolved Hide resolved
u3ks and others added 2 commits June 17, 2024 13:37
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
momepy/graph.py Outdated Show resolved Hide resolved
@martinfleis
Copy link
Member

One more thing, can you update API docs? Removing the old one and adding the new to the proper section.

@jGaboardi jGaboardi added the enhancement New feature or request label Jun 17, 2024
@martinfleis martinfleis merged commit 51e561c into pysal:main Jun 17, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants