Use flat adjacency and dense degree buckets for core decomposition - #1673
Use flat adjacency and dense degree buckets for core decomposition#1673Jesse 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. |
Summary
Speed up
core_numberby replacing node-keyed working maps, neighbor-set cloning and degree sorting with dense indices, flat adjacency and degree buckets. Public APIs, node IDs, result order and neighbor semantics are preserved. Includes regression tests, benchmarks and a release note.Performance
Against upstream
e02dc7c, median call times improved in all 84 tested configurations per interface: 1.63–7.42× faster in Rust and 1.44–5.74× through Python. Tests covered 21 fixtures, compact/fragmented indices and one/eight Rayon threads on a shared ARM64 Mac (Rust 1.97.1, Python 3.12.12).Memory tradeoff: whole Python benchmark-process peak RSS increased from 466–469 MB to 535–538 MB. This includes setup and retained outputs; it is not algorithm-only memory.
Full benchmark results, methodology and build provenance.
Validation
Local macOS checks passed: 435 Rust tests, 95 doctests and 2,391 Python tests (nine existing Linux-only skips), plus Clippy, rustfmt and Ruff. Python used a verified runtime-equivalent extension built before comment-only cleanup.
Regressions cover all 4,096 loop-free four-node directed graphs against an independent oracle, sparse/reused IDs and repeated calls.
AI/LLM disclosure
The content of the PR has been generated by Codex and I have then reviewed it.