Skip to content

refactor(req): model items as totally ordered values - #244

Merged
tisonkun merged 7 commits into
apache:mainfrom
tisonkun:codex/req-generic-policies
Aug 29, 2026
Merged

refactor(req): model items as totally ordered values#244
tisonkun merged 7 commits into
apache:mainfrom
tisonkun:codex/req-generic-policies

Conversation

@tisonkun

@tisonkun tisonkun commented Aug 29, 2026

Copy link
Copy Markdown
Member

Summary

  • require only Clone + Ord for the in-memory REQ algorithm, matching the paper's totally ordered item universe
  • keep ReqValue limited to optional serialization, so custom ordered types can use in-memory APIs without a codec
  • provide ReqFloat<f32/f64> for non-NaN numeric ordering, with signed zeros equal and infinities ordered numerically
  • document the floating-point requirement at the module entry point and use ReqFloat throughout the user-facing REQ behavior tests
  • preserve the existing C++/Java float wire format and byte-for-byte fixtures

Rationale

Relative Error Streaming Quantiles defines REQ over a totally ordered universe. The algorithm sorts compaction ranges and its rank definition relies on that same order. PartialOrd plus a self-comparison check does not express this requirement: a value can compare with itself while remaining incomparable with another value.

The generic core therefore uses Rust's standard Ord contract directly. There is no REQ-specific comparison trait, comparator policy, default policy type, or external dependency. Primitive floats do not implement Ord because NaN is unordered, so ReqFloat rejects NaN at construction and makes the algorithm's ordering precondition explicit.

Adapter scope

ReqFloat intentionally forwards only invariant-preserving, high-value read operations:

  • into_inner for ownership recovery
  • immutable Deref for float methods and borrowed access
  • transparent Debug and Display

It does not implement DerefMut, unconditional conversion from a primitive float, or arithmetic operators. Those APIs could bypass the NaN check or require a large family of potentially panicking forwarding implementations when an operation produces NaN. Callers can extract the primitive explicitly when they need unrestricted float behavior.

API impact

use datasketches::req::{ReqFloat, ReqSketch};

let mut sketch = ReqSketch::default();
sketch.update(ReqFloat::<f64>::new(1.5)?);
# Ok::<(), datasketches::error::Error>(())
  • ReqSketch<T> and SortedView<T> require T: Clone + Ord.
  • Bare f32/f64 sketches become ReqSketch<ReqFloat<f32/f64>>.
  • Serialization additionally requires ReqValue; ReqFloat uses the same raw float bytes and rejects NaN images during deserialization.
  • REQ remains unreleased, so the change is reflected directly in the unreleased API and changelog.

Size

No dependency or policy layer was added. The complete PR is 584 insertions and 537 deletions relative to main (net +47 lines); the net addition is module/API documentation plus explicit float use in existing tests.

Tests

  • REQ behavior, accuracy, bounds, merge, property, sorted-view, and iterator integration tests use ReqFloat<f32/f64>
  • custom Clone + Ord values use in-memory APIs without implementing ReqValue
  • adapter coverage checks NaN rejection, signed-zero equality, and infinities
  • existing C++/Java fixtures still deserialize and reserialize byte-for-byte
  • cargo x check
  • cargo x test
  • cargo x lint

@tisonkun tisonkun changed the title refactor(req): separate ordering from item codecs refactor(req): decouple item storage from serialization Aug 29, 2026
@tisonkun tisonkun changed the title refactor(req): decouple item storage from serialization refactor(req): model items as totally ordered values Aug 29, 2026
@tisonkun
tisonkun marked this pull request as ready for review August 29, 2026 05:38
@tisonkun
tisonkun merged commit 22a766d into apache:main Aug 29, 2026
10 checks passed
@tisonkun
tisonkun deleted the codex/req-generic-policies branch August 29, 2026 05:39
@tisonkun

Copy link
Copy Markdown
Member Author

in sync @pmcgleenon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant