Skip to content

Fix NaN-aware top/bottom N parity across engines#861

Merged
polakowo merged 1 commit into
polakowo:masterfrom
emiliolugo:perf/top-bottom-n-argpartition
Jul 3, 2026
Merged

Fix NaN-aware top/bottom N parity across engines#861
polakowo merged 1 commit into
polakowo:masterfrom
emiliolugo:perf/top-bottom-n-argpartition

Conversation

@emiliolugo

@emiliolugo emiliolugo commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make top_n/bottom_n NaN-aware in records.nb without dtype promotion, preserving deterministic tie order for non-NaN data
  • align Rust top_n_mapped_mask/bottom_n_mapped_mask semantics with Numba for NaN-containing columns (including n > n_valid and all-NaN cases)
  • add regression/parity coverage for NaN selection, overflow behavior, and tie-order determinism

Backward compatibility

  • non-NaN behavior is preserved; changes apply to NaN-containing inputs where selection is now explicit and engine-consistent

Test plan

  • uv run --python 3.11 --extra test pytest tests/test_records.py -k "top_bottom_n_mask_with_nan or top_bottom_n_mask_with_all_nan_column or top_bottom_n_mask_tie_order"
  • uv run --python 3.11 --extra test pytest tests/test_engine.py -k "top_n_mapped_mask_nan or bottom_n_mapped_mask_nan or top_bottom_n_mapped_mask_nan_overflow" (Rust-gated tests skip when rust package is unavailable)
  • uv run --python 3.11 --extra test pytest

Co-authored by Cursor

Align top_n/bottom_n selection semantics for NaN-containing mapped arrays across engines while preserving deterministic tie order and non-NaN behavior. Add targeted regression and parity tests for NaN cases, overflow selection, and tie-order determinism.

Co-authored-by: Cursor <cursoragent@cursor.com>
@polakowo polakowo merged commit 0a2f9ab into polakowo:master Jul 3, 2026
25 checks passed
@polakowo

polakowo commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Merged, thanks!

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