Use flat adjacency and dense degree buckets for core decomposition - #1673
Jesse Wright (jeswr) wants to merge 2 commits into
Conversation
Build per-call dense working state using fixed result-map key positions. Deduplicate neighbors with row markers in a contiguous adjacency buffer, then peel degree buckets without cloning a bucket-offset buffer. Preserve the public Rust and Python APIs, original node IDs, result order, and existing reciprocal-neighbor semantics. Include independent Rust and Python regressions, benchmark controls, and a release note.
|
|
|
I have signed the CLA. |
There was a problem hiding this comment.
🟡 Changes recommended
The updated Rust module imports an unused symbol (InitWithHasher), which can fail CI under warnings-as-errors and should be removed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR accelerates core_number/core decomposition by switching the Rust implementation from node-keyed hash maps + neighbor-set cloning to dense working indices with flat adjacency storage and degree buckets, while aiming to preserve public API behavior (node IDs and result order). It also adds new regression tests, a Rust benchmark, a Python benchmark helper, and a release note documenting the change.
Changes:
- Reworked
rustworkx_core::connectivity::core_numberto use dense indices, contiguous adjacency, and bucket-based peeling. - Added regression tests (Rust + Python) covering sparse/reused IDs, repeated calls, reciprocal arcs, and legacy neighbor-set behavior.
- Added benchmark tooling (Rust
cargo benchtarget + standalone Python benchmark) and a release note.
File summaries
| File | Description |
|---|---|
| tools/bench_core_number.py | Adds a standalone Python benchmark that times the installed extension and validates results per sample. |
| tests/graph/test_core_number.py | Adds PyGraph regression tests for reused IDs and repeated calls with edge changes. |
| tests/digraph/test_core_number.py | Adds PyDiGraph regression tests for reused IDs, reciprocal arcs, repeated calls, and neighbor-set compatibility. |
| rustworkx-core/src/connectivity/core_number.rs | Replaces the core algorithm implementation with dense adjacency + degree buckets; adds extensive Rust regression tests. |
| rustworkx-core/Cargo.toml | Registers the new core_number benchmark target. |
| rustworkx-core/benches/core_number.rs | Adds a reproducible benchmark binary validating correctness outside timed regions and emitting CSV samples. |
| releasenotes/notes/core-number-dense-state-60fb188fd560af1d.yaml | Documents the optimization and preserved semantics in release notes. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
So this is conceptually interesting. I do think there might be low-hanging in older code that is slow and could be optimized. With that being said, this is currently not mergeable. Read https://github.com/Qiskit/rustworkx/blob/main/CONTRIBUTING.md (or tell your agent to follow it more faithfully). Do you see any existing benchmarks in the repository? No. Do you see a CI job for running stuff in the Once you fix that I can take a deeper look. |
|
How I suggest proceeding is: Make a script with For Python: same spirit, but you'll need to versions. Make a Python script, use There's no need to check in the files, just give a gist or a comment with the benchmark. I will take a look next week. |
|
I've read and addressed your review comments, and I have personally reviewed the changes made by Codex. Removed the benchmark files and their Cargo registration. The standalone Criterion and uv comparisons against 0.18.1 are here, with commands and results. Both scripts pin the tested PR commit and use the same fixtures for both versions. |
Summary
Speed up
core_numberusing dense working indices, flat adjacency and degree buckets. Public APIs, node IDs, result order and neighbor semantics are preserved. Includes regression tests and a release note.Performance
Standalone Criterion and
uvcomparisons against 0.18.1 improved all 12 selected configurations in both version orders: 4.10–6.68× in Rust, 2.27–5.78× through Python. See the standalone benchmarks for scripts, commands, results and build limitations.The earlier comparison against the PR base covers 84 configurations per interface. It also records a memory tradeoff: whole Python benchmark-process peak RSS rose from 466–469 MB to 535–538 MB, including setup and retained outputs. This is not algorithm-only memory.
Validation
Local checks passed: 435 Rust tests, 95 doctests and 2,391 Python tests (nine existing Linux-only skips), Clippy, rustfmt and Ruff. Python used a verified runtime-equivalent extension built before comment-only cleanup. Removing benchmark tooling leaves the algorithm and tests unchanged.
AI/LLM disclosure