Skip to content

Pair rows that differ only in a zero's sign alike in both runs - #142

Merged
alxmrs merged 1 commit into
mainfrom
m1/simulation-signed-zero-rows
Oct 31, 2026
Merged

alxmrs merged 1 commit into
mainfrom
m1/simulation-signed-zero-rows

Conversation

@alxmrs

@alxmrs alxmrs commented Oct 31, 2026

Copy link
Copy Markdown
Member

🪤 Claude (fuzz adversary)

Closes #138.

The soak of 0.2.0rc1, run 41230939342 with #134's generators over about 620,000 cases, failed 14 times, on 11 seeds. Every failure was the test's own mistake, not einfold's. Each was a pair of rows with the same keys whose values were 0.0 and -0.0. A domain with duplicate keys gives such pairs when its rows hold signed zeros and a fill copies them, as COALESCE(s.v, d.v) does. #134's finite values make those zeros common. The inexact comparison pairs einfold's rows with the host's after sorting them, and its sort took the two zeros as equal. So each side kept the order its own plan made, and a 0.0 was compared with a -0.0.

Decisions to review

  • Values sort in IEEE 754's total order (check::rows), which puts -0.0 before 0.0. It still orders NULL first, then NaN, then the rest. The comparison of values is unchanged: a zero's sign must still match.

How it is tested

All 11 seeds pass, and the first failed before this change. 2,000 cases from seed 777000 pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SCMAk87qC9VKCk22P7HDH1

The soak of 0.2.0rc1 (#138) failed 14 times on rows with the same keys
whose values were 0.0 and -0.0, as a domain's duplicate keys give with
finite values: the sort that pairs rows took the zeros as equal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SCMAk87qC9VKCk22P7HDH1
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 620e0b8 and 4b222be.

Files selected for processing: 1
  • crates/einfold-datafusion/tests/simulation/check.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@alxmrs

alxmrs commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

🪤 Claude (fuzz adversary)

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@alxmrs
alxmrs merged commit 6a15058 into main Oct 31, 2026
7 checks passed
@alxmrs
alxmrs deleted the m1/simulation-signed-zero-rows branch October 31, 2026 21:07
alxmrs added a commit that referenced this pull request Oct 31, 2026
🪭 Claude (einfold)

This sets einfold for Python's version to `0.2.0rc2`. Once it merges and
`python-v0.2.0rc2` is tagged on it, the release candidate goes to PyPI.
Users then get it with `pip install --upgrade --pre
"einfold[datafusion]"`, and the plain install keeps 0.1.0. It's for
xarray-sql's acceptance testing. xarray-sql is an XQL Systems project
that queries Xarray datasets with SQL. xarray-sql passed 0.2.0rc1
(xqlsystems/xarray-sql#285).

Since rc1:
- #143 computes a chain of three products, such as `SUM(a.v * b.v *
c.v)` over three joined tables, as two dense products in the cheaper
order. It reorders only where a float guard shows no sum can overflow,
and otherwise runs #131's fold or the host's plan. A chain of matrix
products is now 3× DataFusion at `n = 32` (it tied in rc1), 22× at 64,
and 450× at 128. DuckDB is slower than DataFusion at every size
measured.
- #140 fixes #136: from Python, functions now run with the session's
configuration, so `to_unixtime` reads text in the session's time zone.
0.1.0 and rc1 read it in UTC.
- #141 checks that einfold changes no built-in function's answer from
Python, under two time zones.
- #134 and #142 extend the simulation, einfold's random comparison
against DataFusion, to rc1's new shapes, and fix its pairing of rows
that differ only in a zero's sign (#138).

Still open for M3: a chain whose last factor is a vector is 0.8×
DataFusion at `n = 32` (4.1 ms against 3.2 ms), and TPC-H is 1.06×
DataFusion's time. Both are the Python bridge's fixed cost per query
(#81, #58). The fused backward pass waits on xqlsystems/ddx#243.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01M96xSez6PZwTmfaGh2eJgS

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
alxmrs added a commit that referenced this pull request Nov 1, 2026
🪭 Claude (einfold)

Sets einfold for Python's version to `0.2.0`. Once this merges, the tag
`python-v0.2.0` publishes it to PyPI, and `pip install --upgrade
"einfold[datafusion]"` gets it without `--pre`. It's the same code as
0.2.0rc2, which xarray-sql acceptance-tested. xarray-sql is an XQL
Systems project that queries Xarray datasets with SQL. The maintainer
approved the release.

Since 0.1.0:
- #129, #131 and #143 add folds with a fill as the domain's default,
regrouped operands, and chains of three products in the cheaper order,
under a float guard. A chain of matrix products is 3× DataFusion at `n =
32`, 22× at 64, and 450× at 128.
- #132 and #140 run DataFusion's default functions in einfold's library,
with the session's configuration. `to_unixtime` reads text in the
session's time zone (#136).
- #141 checks that einfold changes no built-in function's answer from
Python. #134 and #142 extend the simulation to the new shapes.

Known issues, for later releases:
- #145 and #146: a `DataFrame` that reads an in-memory table fails once
its `SessionContext` is dropped. The cause is in datafusion-python; the
workaround is to keep the context alive.
- The Python bridge's fixed cost per query (#81, #58): a chain whose
last factor is a vector is 0.8× DataFusion at `n = 32`, and TPC-H is
1.06× DataFusion's time.

## Decisions to review

None.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01M96xSez6PZwTmfaGh2eJgS

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

Nightly fuzz: 14 property failure(s) in the simulation soaks

1 participant