Skip to content

Pair the simulation's rows by total order, so -0.0 and 0.0 don't swap - #139

Closed
alxmrs wants to merge 1 commit into
mainfrom
m1/simulation-zero-order
Closed

alxmrs wants to merge 1 commit into
mainfrom
m1/simulation-zero-order

Conversation

@alxmrs

@alxmrs alxmrs commented Oct 31, 2026

Copy link
Copy Markdown
Member

🪭 Claude (einfold)

The simulation, einfold's random differential test against plain DataFusion, compares results row by row after sorting them by keys and then values. It sorted values with partial_cmp, which treats -0.0 and 0.0 as equal. Where two rows have the same keys and values that differ only in a zero's sign, as when a domain repeats a key and the query fills both rows from the domain's own 0.0 and -0.0, the stable sort kept them in arrival order, which differs between einfold's run and the host's. The comparison then paired each with the other's row, and reported a sign difference that isn't one. Nothing changes for users.

This is all 14 failures of the soak in #138 (11 seeds). With values sorted by total_cmp, which orders -0.0 before 0.0, every one of the 11 seeds passes at the soak's commit. In the first, einfold ran the host's own plan (host_plans=1), so the two results were the same rows. #134's new generators are what made such domains common.

Decisions to review

  • Sort by total_cmp, not by bits or as a multiset (rows). Values still pair by order within a key, so rows that agree within the float bound still pair up. The comparison of zeros themselves is unchanged: a zero must still have the host's sign.

How it is tested

The 11 seeds from #138 replayed at 8ec90e5 with this change, and the default run passes.

Closes #138.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M96xSez6PZwTmfaGh2eJgS

@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • 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 (einfold)

Closing in favor of #142, the same fix from the simulation's owner, opened while this one was.

@alxmrs alxmrs closed this Oct 31, 2026
@alxmrs
alxmrs deleted the m1/simulation-zero-order branch October 31, 2026 20:33
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