Skip to content

A/B allocator summary admits unscored pairs and does not forward run seeds #228

Closed

Description

@Birfy

Bug and scenario

Verified on merged main 8585b3fee5841ebe3389371838f8c9ccb0fd4bc6 (Python 3.12.14). Two remaining A/B harness findings from #204 reproduce through the actual bench.ab_allocator.main path. A run can report a complete paired comparison after one arm's final scoring failed, and the allocator's exploration seed does not follow the requested run seed.

This concerns benchmark accounting and reproducibility. It does not dispute the earlier decision/feedback attribution fixes or claim a measured model quality/cost benefit.

Offline reproduction

Run from the repository root at the commit above with the project dependencies installed. All workload, model and dataset-facing functions are replaced by local stubs; this makes no provider or dataset calls.

PYTHONPATH=. python - <<'PY'
import contextlib
import io
from types import SimpleNamespace

import bench.ab_allocator as ab


class Workload:
    evolve_kwargs = {}
    tasks = []
    reward = None
    agent = strategy = None

    def test_eval(self, result):
        if result.fail_score:
            raise RuntimeError("mock scoring failure")
        return 0.625


ab.confirm = lambda args: True
ab.completion_for = lambda *a, **k: None
ab._finer = lambda *a, **k: Workload()


class StubAllocator:
    created = []

    def __init__(self, seed=0, **kwargs):
        self.seed = seed
        self.n_skipped = self.n_observed = self.n_counterfactual = 0
        type(self).created.append(seed)


ab.ValueBudgetAllocator = StubAllocator


# A test-evaluation failure leaves evolve's result.error unset.
def fake_evolve(*args, usage, allocator=None, seed=None, **kwargs):
    usage.record(prompt_tokens=20 if allocator else 10)
    return SimpleNamespace(
        error=None,
        final_reward=0.5,
        rollouts=1,
        outcomes=lambda: {"committed": 1},
        fail_score=allocator is not None,
    )


ab.evolve = fake_evolve
out, err = io.StringIO(), io.StringIO()
with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):
    rc = ab.main(
        ["--dataset", "finer", "--seeds", "7", "--max-tokens", "100", "--yes"]
    )

print("scoring-failure rc=", rc)
print(out.getvalue())
print("stderr:", err.getvalue().strip())


# Both scores succeed; capture evolve's seed versus the allocator's seed.
seen = []


def fake_evolve_seed(*args, usage, allocator=None, seed=None, **kwargs):
    usage.record(prompt_tokens=10)
    if allocator is not None:
        seen.append((seed, allocator.seed))
    return SimpleNamespace(
        error=None,
        final_reward=0.5,
        rollouts=1,
        outcomes=lambda: {"committed": 1},
        fail_score=False,
    )


ab.evolve = fake_evolve_seed
StubAllocator.created.clear()
out, err = io.StringIO(), io.StringIO()
with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):
    rc = ab.main(
        ["--dataset", "finer", "--seeds", "17,42", "--max-tokens", "100", "--yes"]
    )

print(
    "seed-forwarding rc=", rc,
    "allocator constructors=", StubAllocator.created,
    "evolve_seed/allocator_seed=", seen,
)
print(out.getvalue())
PY

Observed scoring-failure summary (columns abbreviated):

scoring failed: RuntimeError
arms: 2 (2 x 1 seeds), 0 failed
off: 1 complete seed, quality 0.625 / 0.625 / 0.625, 10 tokens
on:  1 complete seed, quality n/a, 20 tokens

The treatment has no quality measurement, but its row and tokens remain in the paired comparison. In a larger run, quality and cost can therefore summarize different successful samples.

Observed seed capture:

allocator constructors = [0, 0]
(evolve_seed, allocator_seed) = [(17, 0), (42, 0)]

Expected: a scoring failure makes the whole seed pair ineligible for both paired quality and paired cost. Failed/incomplete spend must still be retained and reported separately rather than silently discarded. Each treatment allocator must receive its corresponding run seed.

Source

Acceptance criteria

  • Record scoring failures explicitly and retain their accrued usage in failure diagnostics.
  • Use exactly the same successfully scored seed pairs for quality and paired-cost comparisons; report excluded/incomplete spend separately.
  • Make an empty measurable comparison explicit, without presenting it as a valid paired result.
  • Forward the run seed to the allocator.
  • Add offline main() regressions for one-arm/both-arm scoring failures, a mixed successful/failed seed set, retained usage, and distinct nonzero seeds.
  • Preserve successful-run behavior and existing evolution/build-failure accounting.

Dependencies and prior evidence

#204 is merged; no other implementation dependency is required. Scope: bench/ab_allocator.py and offline regression tests. ModelScope cache bypass is a separate loader concern; weak async-fusion assertions are a coverage limitation, not a demonstrated runtime defect in this report.

Originally reported in the exact-head follow-up review. Reproduced again on the merged production tree. Five nearby existing offline tests passed, including arm wiring and default ModelScope caching, so those passes do not cover these paths.

Dedup checked current open work and all-state searches for allocator, paired seeds, scoring, ab_allocator and cache concerns; no separate issue or fix PR tracks these two harness defects.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions