Skip to content

DDIR explain: demand is a set, a min demands what may decide it, and the restricted run reads demanded rows whole - #986

Merged
frankmcsherry merged 3 commits into
TimelyDataflow:master-nextfrom
frankmcsherry:ddir-explain-demand-sets
Oct 21, 2026
Merged

frankmcsherry merged 3 commits into
TimelyDataflow:master-nextfrom
frankmcsherry:ddir-explain-demand-sets

Conversation

@frankmcsherry

Copy link
Copy Markdown
Member

Three explain soundness fixes, found by the differential testing (findings 5 and 6, plus one found while sweeping for them). With them, the explain oracle passes with the negation and non-monotone skips removed. Each commit has a regression test in tests/explain.rs that fails without it. +93/−25, all in explain/ and its tests.

1. Demand is a set (e6b9824).

  • The bug: demand reaching a collection along two paths was summed. With negative counts on one path, the two cancelled and the explanation came out empty. The minimal case is a negated collection joined with its own swap.
  • Why it's more than negation: negative counts occur inside every loop too, because each round's records hold that round's changes.
  • The fix: a collection maps data to diffs, and an explanation names the data whose diffs must be accurate, so demand's counts mean nothing and must never cancel. Demand, and every table demand is looked up in (reference run, restricted run, sources), is now read as a set, through Sb::presence, renamed from distinct_full.
  • Test: negated_fanout_explanations_sufficient.

2. A min demands the inputs at or below its demanded value (50fafd8).

  • The bug: a min's keyed lookup kept only the inputs whose value equals the demanded value. When the restricted run raised a key's minimum (the inputs achieving the lower value left out), demand asked for the inputs achieving the wrong, raised value, and never the reference's lower ones. The minimal case is label propagation, then a count per label.
  • The fix: narrowing keeps the inputs at or below the demanded value. That covers both ways the restricted run can be wrong: a raised minimum takes in the reference's lower achieving inputs, and a lowered one takes in the inputs that lowered it. It's a single predicate.
  • Test: min_then_count_explanations_sufficient, plus the updated value-contract unit test.

3. The restricted run reads demanded rows whole (5179498).

  • The bug: the restricted run's inputs, and each demand set's closure, matched demanded rows to source rows by key alone. Demanding one row brought in every source row at that key, at inflated counts.
  • The fix: they now match whole rows (Sb::restrict).
  • Test: restricted_rows_keep_their_counts.

Validation

  • Sweep: with the DDIR: a differential-testing sweep over random lambda programs #980 harness copied in locally, keeping only the timed and quadratic skips, 15,000 seeds with 0 failures. The earlier failing cases 3003503, 3004641, 3004659, 3005430 and 9006406 all pass.
  • Checks: the release explain suite with ignored tests (29/29), workspace tests, doc tests, clippy, the LDBC smoke and SNB suite at 1 and 4 workers plus maintained mode.

Cost: release explain suite, instructions retired: 423.1G on master-next, 567.4G here (+34%). This was accepted for correctness.

Follow-up once #980 lands: remove the harness's shape skip for these findings, so case.explainable = !g.timed && !exploded;, and update the Case::explainable doc.

🤖 Generated with Claude Code

https://claude.ai/code/session_019XsakwFGwtESwZ1gSYwG4o

frankmcsherry and others added 3 commits October 20, 2026 21:05
A collection maps data to diffs, and an explanation names the data whose
diffs must be accurate, so demand's counts mean nothing. Explain still
combined demand by its counts. A lookup multiplied demand by the count of
the host-form row it found, and the host forms of a collection below a
negation, or inside a loop (whose records hold each round's changes), have
negative counts. So demand reaching one collection by two paths could come
back as +1 and -1, cancel in the site's concat, and leave a row's
explanation empty; a lone negative contribution was dropped as well.

Demand and every host form, witness and forward, sites and sources, are
now read as sets (`Sb::presence`, which was `distinct_full`): each row
whose count is not zero, once. Pair tables then carry no negative counts,
and demand never cancels.

tests/explain.rs `negated_fanout_explanations_sufficient` reads a negated
collection directly and through a map; it fails without this.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019XsakwFGwtESwZ1gSYwG4o
A min's output at a key is decided by the inputs at or below it. The keyed
lookup narrowed a demanded min row to the inputs whose value equals it,
which is right for a row the reference produced and the restricted run
agrees with. It misses both ways the restricted run can disagree:
- the restricted run raised the value, its lower inputs left out: the
  narrowing demanded the restricted run's achieving inputs and never the
  reference's, so the key's output was never made as in the reference. A
  label that rose to the queried label then joined a later count's group,
  and that count's explanation was insufficient;
- the restricted run lowered the value: the inputs that lowered it were
  never demanded, so they were never corrected.

The narrowing now keeps the inputs at or below the demanded value
(`ky_data_le`), from both runs as before. The value contract test covers
the least value and a higher one.

tests/explain.rs `min_then_count_explanations_sufficient`, label
propagation by `min` and then a count per label, fails without this.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019XsakwFGwtESwZ1gSYwG4o
The restricted run's inputs are the sources restricted to the demand sets.
They were restricted by a semijoin on the key alone, so demanding one row
fed the restricted run every source row at that key, each once per
demanded row at it. For an input with several values at a key, the
restricted run then computed on rows and counts the demand did not name:
a count over the demanded rows came out as a count over the whole key, and
the restricted run could agree with the reference by accident, so the
inputs that make it agree were never demanded. Each demand set's closure
kept demanded rows by key alone the same way.

Both now match whole rows (`Sb::restrict`), and keep the source's counts.

tests/explain.rs `restricted_rows_keep_their_counts`, a count over an input
with several values at a key, its difference with the input, and a min,
fails without this.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019XsakwFGwtESwZ1gSYwG4o
@frankmcsherry
frankmcsherry merged commit bc57e12 into TimelyDataflow:master-next Oct 21, 2026
6 checks passed
@frankmcsherry
frankmcsherry deleted the ddir-explain-demand-sets branch October 21, 2026 01:22
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