Skip to content

Fix/median total order - #1

Merged
ysBach merged 3 commits into
mainfrom
fix/median-total-order
Sep 5, 2026
Merged

Fix/median total order#1
ysBach merged 3 commits into
mainfrom
fix/median-total-order

Conversation

@ysBach

@ysBach ysBach commented Sep 5, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are well-scoped, preserve documented preconditions (no-NaN “valid” buffers), and add comprehensive tests for the corrected edge cases.

Pull request overview

This PR improves floating-point order-statistics correctness by enforcing a total float ordering (not partial_cmp) and by computing the even-length median midpoint in an overflow-safe way, with added regression tests for edge cases (extremes, infinities, signed zero).

Changes:

  • Switch order-statistics comparisons to a total float order via Float::total_cmp (supporting -0.0 vs 0.0 and infinities deterministically).
  • Replace (lower + upper) / 2.0 with an overflow-safe midpoint(lower, upper) in the even-length median path.
  • Add targeted tests covering overflow safety, infinities, and signed-zero semantics; ignore .worktrees/ in Git.
File summaries
File Description
src/reducers_1d.rs Use total ordering for float comparisons; introduce overflow-safe midpoint for even median; add extensive regression tests.
src/finite.rs Add Float::total_cmp (default + f32/f64 overrides) to standardize total ordering across kernels.
.gitignore Ignore .worktrees/ directory.
Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ysBach
ysBach merged commit d87f00d into main Sep 5, 2026
1 check passed
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.

2 participants