Fix kahns algo sparse vertex performance - #15072
Conversation
for more information, see https://pre-commit.ci
|
@cclauss kindly review |
1 similar comment
|
@cclauss kindly review |
|
@mindaugl |
cclauss
left a comment
There was a problem hiding this comment.
Please add a benchmark that proves that the proposed modification could save significant runtime.
Sure I will do that |
|
What are the benchmark results on your machine? |
Here are the benchmark results on my machine comparing the pre-optimization implementation (list.pop(0)) against the current implementation (deque.popleft()) for topological_sort() on a 30,000-vertex DAG across 5 runs: Benchmark results for topological_sort with 30000 vertices over 5 runs: Pre-optimization (list.pop(0)): 0.29597 seconds Speedup ratio: 6.00x faster The benchmark directly measures topological_sort() on the exact same graph input and programmatically asserts that both implementations return valid, complete topological sorts. |
Describe your change:
Fixes the
O(N)queue operation caused bylist.pop(0)by usingcollections.dequeandqueue.popleft(), ensuring constant-time dequeue operations and preserving the expectedO(V + E)time complexity of Kahn's topological sort algorithm.Fixes an
IndexErrorfor sparse/non-contiguous integer vertex IDs by changingindegreefrom a list to a dictionary keyed by the graph vertices.Adds doctests covering topological sorting with sparse/non-contiguous integer vertex IDs.
Adds doctests covering cycle detection with non-contiguous vertex IDs.
Add an algorithm?
Fix a bug or typo in an existing algorithm?
Add or change doctests?
Documentation change?
Checklist:
Fixes #15071