Skip to content

fix: spread the quantile sample remainder instead of dropping it - #192

Merged
SrGonao merged 1 commit into
EleutherAI:mainfrom
abhishekcpp007:abhishekcpp007/fix-quantile-sample-remainder
Nov 2, 2026
Merged

SrGonao merged 1 commit into
EleutherAI:mainfrom
abhishekcpp007:abhishekcpp007/fix-quantile-sample-remainder

Conversation

@abhishekcpp007

@abhishekcpp007 abhishekcpp007 commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What this fixes

split_quantiles (delphi/latents/samplers.py) computes one per-quantile count for
the whole loop:

samples_per_quantile = n_samples // n_quantiles

So n_samples is honoured only when it divides by n_quantiles. Otherwise the
remainder is dropped, and when n_samples < n_quantiles the count is 0 and the
function returns nothing at all.

Nothing warns in that case. The shortfall log is guarded by
len(quantile) < samples_per_quantile, and no quantile has fewer than zero
examples, so it never fires.

Measured against the current main with 1000 examples:

n_samples n_quantiles returned today after
50 10 50 50
40 10 40 40
25 10 20 25
9 10 0 9
5 10 0 5
1 10 0 1
50 23 46 50
100 7 98 100

The sharp edge is SamplerConfig. n_examples_test=50 and n_quantiles=10 are the
defaults and divide cleanly, so this is invisible on a default run. Pass
--n_examples_test 5, or raise --n_quantiles past n_examples_test, and the latent
is explained and then scored on an empty test set. --n_examples_train 25 quietly
shows the explainer 20 examples.

Fix

Take each quantile's count as the difference of two cumulative counts:

samples_per_quantile = (n_samples * (i + 1)) // n_quantiles - (n_samples * i) // n_quantiles

The counts sum to n_samples exactly, and the n_samples % n_quantiles extra samples
land in quantiles spaced across the range rather than bunched at the top, which would
bias the sample toward the highest-activating examples. For n_samples=25, n_quantiles=10
that is [2, 3, 2, 3, 2, 3, 2, 3, 2, 3]; for 5, 10 it is [0, 1, 0, 1, 0, 1, 0, 1, 0, 1].

A request that already divided evenly gets the identical count per quantile, so every
default config and every config in tests/ and tests/e2e.py samples exactly what it
sampled before. train_type="mix" now reaches n_train as well: with the defaults it
was taking 38 of the 40 it asked for.

Which quantile an example belongs to doesn't change. The quantile slices and the
exclude check are untouched.

Tests

In tests/test_latents/test_sampler.py:

  • test_split_quantiles_returns_as_many_as_asked_for, parametrized over the pairs in
    the table above. The six non-dividing pairs fail on main
  • test_split_quantiles_samples_stay_in_their_own_quantile — the remainder changes how
    many come from a quantile, not which quantile an example is in
  • test_fewer_test_examples_than_quantiles — the symptom through sampler:
    n_examples_test=5 with n_quantiles=10 leaves record.test empty on main

The existing test_simple_cache asserts len(record.test) <= n_examples, which is why
n_examples=5, n_quantiles=10 passes today on an empty result. I left that assertion
alone rather than tightening someone else's test in this PR.

The 1000-example fixture the file already builds is now a small helper, since four tests
need it.

CI, and why these tests can't run yet

main has been red since 30 Oct, stopping during collection before any test runs:

ImportError: cannot import name 'ActivatingExample' from 'delphi.latents' (unknown location)
Interrupted: 5 errors during collection

One of the five modules is tests/test_latents/test_sampler.py, which is where these tests
live, so this PR's tests cannot run until #185 lands. Nothing here touches that.

I ran the build on my fork twice to separate the two. This branch alone reproduces the same
collection error, with ruff check clean and none of the repo's 64 pyright errors in either
file I touched. With #185 cherry-picked on top of this branch the suite is green:

287 passed, 29 warnings in 98.44s

(run 41433109283), against
278 passed on #185's own run
(41318281754). The difference
is exactly the nine tests added here, and none of the existing 278 broke.

Happy to rebase this on #185 whenever that suits you.

What I ran locally

  • ruff check on both files, clean
  • black on both files (pre-commit's pinned 26.3.0)
  • I could not install the package here (no torch), so I did not run pytest. Saying that
    rather than reporting a pass I didn't see. The count arithmetic and the slicing are plain
    Python, so I ran both versions of the function side by side over n_samples 0 to 60 across
    n_quantiles in 1, 2, 3, 7, 10, 23: every total matches the request, no sample leaves its
    quantile, and exclude still holds. CI is the real run.

`split_quantiles` took `n_samples // n_quantiles` from every quantile, so a
request that didn't divide evenly came back short and one smaller than
`n_quantiles` came back empty. `--n_examples_test 5` with the default
`--n_quantiles 10` asked for five scoring examples and produced none, with no
warning: the per-quantile count was 0, which is never fewer than the examples
available, so the "fewer than" log never fired either.

Each quantile's count now comes from the difference of two cumulative counts,
which sums to `n_samples` exactly and puts the remainder in quantiles spaced
across the range rather than bunched at the top. A request that already divided
evenly, every default among them, takes the same examples it did before.
@SrGonao
SrGonao merged commit d97c1e1 into EleutherAI:main Nov 2, 2026
2 checks passed
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