Conversation
Keep the existing list-consistent iterator unchanged. Add allocation-free slot lookup, invariant tests, deterministic distribution diagnostics, and paired Criterion measurements with documented tradeoffs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The novel permutation construction and its probabilistic guarantees warrant final expert human validation despite strong tests and documentation.
Review effort: Balanced
Findings: None
What changed in this PR
Adds experimental cycle-consistent replica selection with direct slot lookup, constant iterator state, extensive validation, and measured comparison against the existing policy.
Changes:
- Adds and exports
VirtualPermutation. - Adds correctness and distribution diagnostics.
- Adds reproducible benchmarks and design/performance documentation.
| File | Description |
|---|---|
src/virtual_permutation.rs |
Implements and tests cycle-consistent selection. |
src/lib.rs |
Exports the new API. |
README.md |
Explains differing membership semantics. |
examples/permutation_diagnostics.rs |
Adds distribution diagnostics. |
docs/virtual-permutation.md |
Documents algorithm and guarantees. |
docs/virtual-permutation-performance.md |
Records methodology and results. |
docs/permutation-design.md |
Distinguishes existing semantics. |
benchmarks/summarize_replica_comparison.py |
Summarizes Criterion results. |
benchmarks/replica_comparison.rs |
Adds paired benchmarks. |
benchmarks/Cargo.toml |
Registers the benchmark. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add an explicitly experimental two-bit variant using the exact existing permutation and its inverse. Preserve existing mappings and the stronger variant, and rerun three-way timing, held-out statistics, and traversal diagnostics. Document both recovered throughput and repeatable small-domain bias. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add allocation-free cycle-consistent replica selection alongside the existing streaming
ConsistentPermutation, preserving its API and mappings:VirtualPermutation: the stronger, one-bit-lift implementation, 24-byte state,1..=u64::MAX, directreplica_at(slot)and non-replayingnth.BalancedVirtualPermutation: an explicitly experimental matched-network comparison requested during review. It calls the exact existing Feistel, with the same seed, mixer, round counts and key schedule, using even-width/two-bit lifts over1..=2^30. It also has 24-byte state. The new inverse shares the round function and reverses that exact key schedule; known-answer vectors guard the existing mapping.Different membership semantics, not drop-in replacements: the existing iterator preserves survivor list order. Both cycle variants preserve each old replica slot except a slot replaced by an appended node, or previously naming a removed last node. All provide distinct nodes and stable
kprefixes. Membership must remain consecutive IDs0..n.The documentation derives uniformity and expected O(k) evaluation under independent uniform ideal permutations and constant-cost forward/inverse calls. Actual finite-key networks are noncryptographic approximations: no exact uniformity, cryptographic security or worst-case O(k) guarantee is claimed. No rings, permutation arrays, duplicate sets, recursive stack or mapping-changing retry limits are used.
Latest three-way measurements
Apple M4 Max, native release build, rustc 1.92.0; same seeded corpus and setup accounting for every method. All three were rerun together: 131 N/k cases each for fresh and prehashed queries, plus constructors, setup-excluded streaming, equal-width collection and single-slot workloads. All comparisons fit the existing domain. Each case has 20 samples; the committed report includes 95% intervals, variance, seeds, environment and commands. The runner produces 1,081 estimates.
Mean ns/query:
Matching the primitive and layer stride removes much of the earlier slowdown: at N=1,000,k=3 the cycle implementation improves from 101.72 to 27.72 ns, about 16% faster than existing. But setup-excluded streaming still favors existing, consistent with the complete-query win coming from avoiding allocation rather than cheaper traversal. The matched mean is below existing in 31/131 fresh cases, including the degenerate N=1 case; small differences are not blanket significance claims. Its worst ratio is 3.45x at N=257,k=8, and full enumeration remains slower. This is not an isolation of the cost of odd halves alone.
Statistical tradeoff — do not treat the matched variant as equivalent
Both primary and disjoint held-out 200,000-key corpora were rerun without tuning the inherited network. The matched variant has repeatable small-domain bias:
For comparison, the N=8 subset statistics are 53.47/59.83 for existing and 47.66/40.41 for the stronger variant. At N=5,slot=4 the matched most frequent node is about 4% above its expected count in both corpora. The same primitive can behave differently after list versus cycle projection. The matched variant remains explicitly experimental, not a silent replacement for the stronger implementation or a claim that the ideal randomness goal is met. Existing/stronger diagnostic output and mappings remain unchanged. Statistical diagnostics are not CI p-value gates or proofs of independence.
Documentation and reproduction
cargo bench -p consistent-choose-k-benchmarks --bench replica_comparison -- --noplotpython3 crates/consistent-choose-k/benchmarks/summarize_replica_comparison.py target/criterioncargo run --release -p consistent-choose-k --example permutation_diagnostics(repeat with-- --held-out)Original two-way results at commit
d51abfbremain documented as historical measurements. Wider u64 cases are explicitly unpaired operation-count diagnostics, not speed comparisons with the old API.Validation
Debug and release crate suites pass: 34 unit tests + 4 doctests in each, plus the separately run operation-count diagnostic. Coverage includes full permutations, prefixes, append/delete consistency, power-of-two/four boundaries, explicit lift/cycle-projection oracles, ideal lift fibers, every supported forward/inverse width and extreme values, unchanged-network known answers, invalid inputs and long walks. All-target/all-feature builds, strict targeted Clippy, workspace formatting and diff checks pass. Rustdoc builds with the pre-existing private-link warning in existing documentation.
All work and benchmarks ran in the isolated worktree. Local commands used the already-installed Command Line Tools via
DEVELOPER_DIR=/Library/Developer/CommandLineToolsbecause selected Xcode's license was unaccepted; no license or system setting was changed. No merge requested.