-
-
Notifications
You must be signed in to change notification settings - Fork 35
ENH: Add parallel implementation for Distance Measures (eccentricity, diameter, radius, center, periphery) #155
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
base: main
Are you sure you want to change the base?
Conversation
Schefflera-Arboricola
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i haven't reviewed the implementation in detail yet but have left a few comments below.
Pls consider taking a look at PR #97 (similar diameter algorithm). And consider cherry-picking relevant commits from that PR and crediting the original author in your commit messages, instead of reimplementing from scratch, to avoid duplication of effort. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these tests necessary? we already run nx tests with ParallelGraph obj
| temp__init__.py | ||
|
|
||
| .DS_Store | ||
| .DS_Storeuv.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like a typo
| .DS_Storeuv.lock | |
| .DS_Store | |
|
|
||
|
|
||
| @nxp._configure_if_nx_active(should_run=nxp.should_run_if_large) | ||
| def diameter(G, e=None, usebounds=False, weight=None, get_chunks="chunks"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see usebounds is not being used in the implementation below-- consider using can_run here. Same for the following algorithms..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your guidance, all done as u said !!
|
Thank you for pointing out PR #97! I reviewed it and noticed that PR #97 implements the specialized iFub4S algorithm for diameter, which is optimized for real-world undirected unweighted graphs. My implementation takes a different approach - it parallelizes the basic eccentricity computation, which then enables parallel versions of diameter, radius, center, and periphery. The two approaches could potentially complement each other - iFub4S for fast diameter-only calculations, and my implementation for when users need multiple distance measures. Would you like me to integrate both approaches, or keep them as separate implementations? |
Description
This PR addresses #82 by adding parallel implementations for several "embarrassingly parallel" distance algorithms. These measures are computationally intensive ($O(V \times (V+E))$), making them ideal candidates for parallelization across node-specific shortest path traversals.
Algorithms added:
Key Features:
joblib.nxp.should_run_if_largeto ensure parallel overhead is only incurred when it provides a meaningful speedup for larger graphs.v) is queried.nx_parallel.algorithms.Testing
NetworkXErroras per sequential behavior).Checklist
n_jobsparameter added to functions (respects global config).ruff format.