feat: bound class sampler - #253
Draft
selmanozleyen wants to merge 3 commits into
Draft
selmanozleyen wants to merge 3 commits into
selmanozleyen wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
==========================================
+ Coverage 91.67% 91.76% +0.09%
==========================================
Files 15 19 +4
Lines 1453 1652 +199
==========================================
+ Hits 1332 1516 +184
- Misses 121 136 +15
🚀 New features to boost your workflow:
|
A class's run was picked uniformly among its runs, then a start uniformly inside it, so every run contributed one equally likely position regardless of size. With a 400-row run beside a 40-row one, each got half the chunks and the short run's rows were drawn 10x as often per row. Class marginals were unaffected, so no share test noticed. The ClassSampler docstring still describes the intended scheme: "a uniform chunk-start within c is drawn per chunk (a prefix-sum lookup maps it to the absolute slice in O(log n_runs))". The sampler worked that way until ec49f9b, where the prefix sum changed from cumulative valid positions to cumulative run counts, leaving searchsorted nothing to resolve against. It was replaced by a direct index plus a second draw. Each run now records how many chunk starts it holds and how many precede it in the table, and one uniform draw per class resolves through those to a run and an offset inside it. Two rng.integers calls become one, so a given seed yields different slices than before. Both class samplers draw through RLEManager.slices_from_classes, so this fixes WeightedClassSampler too, and the new tests run over both.
selmanozleyen
force-pushed
the
feat/bound-class-sampler
branch
from
September 18, 2026 23:54
be22c7a to
ab1843d
Compare
selmanozleyen
force-pushed
the
feat/bound-class-sampler
branch
3 times, most recently
from
September 19, 2026 00:55
89da394 to
11ef92c
Compare
selmanozleyen
force-pushed
the
feat/bound-class-sampler
branch
6 times, most recently
from
September 19, 2026 16:04
3c94033 to
edbc769
Compare
Replays another class sampler's per-batch class schedule against a second annotation column, so one pass reads two annotations in step with each other. It takes a ClassSampler or another BoundClassSampler -- so these chain -- and rejects WeightedClassSampler, whose batches mix classes by design. The shared machinery moves into two private bases in _class_sampler.py: _RunClassSampler holds the constructor, the remainder handling and the window loop, and _ScheduledClassSampler adds the per-batch schedule that makes a sampler replayable. WeightedClassSampler drops its near-duplicate _iter_requests and overrides only the two policy hooks, which also removes the Liskov problem of it subclassing ClassSampler while batching differently. BoundClassSampler keeps a private copy of its inner sampler, so nothing the caller does to their own instance afterwards can desynchronise the schedule; its rng setter forwards to that copy, so re-assignment -- as DistributedSampler does per rank -- still reaches it.
DistributedSampler stored the sampler it wrapped by reference and then sharded its mask on every n_batches/validate/_sample call and re-spawned its rng once. Wrapping therefore left the caller's own sampler permanently rank-sharded, so reusing it for eval read 1/world_size of the data. Shard a copy instead. The class samplers reused one row-id buffer for a whole pass and reshuffled it in place. split_given_size is np.split, which returns views, so every window handed out the same arrays: list(sampler.sample(n)) showed the last window's permutation throughout, and a caller writing into one window's split corrupted the next. Allocate per window. A mask assigned part-way through a pass was reported by .mask but never read from, because a pass fixes all of its slices when it starts. Refuse it. class_weights given as a pandas Series was converted positionally, discarding the index, so weights silently landed on the wrong classes. Reject it and say how to align it. All four are pre-existing in main and reproduce identically on every branch.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Putting it here to share my WIP only. Will fill the body. I tried to split this into nice 3 commits for easier review
update: it is in ai draft state atm.