Repository navigation
Conversation
num_batches was computed with ceil() when drop_last_batch=True and floor() when False -- the opposite of the documented behaviour (drop the partial last batch -> floor, keep it -> ceil) and of what batch() does. With drop_last_batch=True the while loop demanded more complete batches than the data could fill and spun forever; with drop_last_batch=False (the mode the genetic optimizer uses) tail samples never made it into any batch, and when batch_size exceeded the dataset size floor() produced zero batches so every sample was dropped. Compute num_batches the right way round, guard the zero-batch case, and replace the quota-based while loop (which consumed samples even for batches it never appended) with a round-robin dealing of each class across the batches: every sample lands in a batch when the partial last batch is kept, and only complete batches are returned when it is dropped. replace=True now tops up incomplete batches by resampling instead of calling random.choices() on an exhausted class.
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.
Issue Link / Problem Description
SingleMetricAnnotation.stratified_batches()computes the number of batches withceil/floorinverted (src/ragas/dataset_schema.py:792-796on main): it usesceil(total_samples / batch_size)whendrop_last_batch=Trueandfloor(...)whenFalse— the opposite of the documented behavior ("If True, drops the last batch if it has fewer samples thanbatch_size" → only complete batches fit → floor) and of what the non-stratifiedbatch()method does. Consequences on main:drop_last_batch=Truenever terminates. Thewhile len(all_batches) < num_batchesloop (src/ragas/dataset_schema.py:808) only appends a batch when it reachesbatch_size; withceildemanding more complete batches than the data can fill (e.g. 20 samples,batch_size=3, or a class running out mid-way atbatch_size=2), the loop spins forever.drop_last_batch=Falsesilently drops samples. This is the mode the genetic optimizer uses (src/ragas/optimizers/genetic.py:282): 20 samples withbatch_size=2return 19; and whenbatch_sizeexceeds the dataset size,floor()yields 0 batches and every sample is dropped (returns[]).replace=Truecrashes: it callsrandom.choices()on an exhausted class's empty list and raisesIndexError.Reproduce on main:
Changes Made
src/ragas/dataset_schema.py(onlystratified_batches):num_batchesthe right way round:floorwhendrop_last_batch=True,ceilwhenFalse— mirroringbatch().batch_size> dataset size withdrop_last_batch=True) → returns[]instead of dividing/looping over zero batches.whileloop (which consumed samples for batches it then refused to append) with a round-robin dealing of each class's samples across the batches: every sample lands in a batch when the partial last batch is kept, and only complete batches are returned when it is dropped. Each batch still gets its proportional share of every class, and batches are still shuffled.replace=Truenow tops up incomplete batches by resampling from the samples already in the batch, instead of callingrandom.choices()on an exhausted class (which raisedIndexError). Thereplacedocstring line is updated to match what the code now does.tests/unit/test_dataset_schema.py: four regression tests (see Testing).Testing
How to Test
New tests in
tests/unit/test_dataset_schema.py(all fail on unpatched main, pass with this fix):test_stratified_batches_without_drop_last_batch_uses_all_samples— 20 samples (11/9 classes),batch_size=2: all 20 samples batched exactly once in 10 batches (main returns 19).test_stratified_batches_with_drop_last_batch_terminates— runs in a subprocess with a timeout since main hangs: 20 samples withbatch_size=2→ 10 complete batches;batch_size=3→ 6 complete batches (partial tail dropped);batch_size=25→[](no complete batch fits).test_stratified_batches_with_fewer_samples_than_batch_size—batch_size=25over 20 samples withdrop_last_batch=False→ one batch with all 20 samples (main returns[], dropping everything).test_stratified_batches_with_replace_fills_batches—batch_size=3,replace=True→ 7 batches all filled to 3 (main returns 6 batches of 2, or raisesIndexError).Commands and results (venv with ragas installed editable; same as
make test/ CI):pytest tests/unit/test_dataset_schema.py -k stratified_batches→4 failed, 18 deselected(AssertionError: assert 19 == 20;Failed: stratified_batches(drop_last_batch=True) did not terminate;assert 0 == 1;assert 6 == 7)4 passedpytest tests/unit/test_dataset_schema.py→22 passed(incl. the pre-existingtest_loader_batch, which also exercisesstratified_batches)pytest tests/unit -k "not slow" --dist loadfile -n auto→3 failed, 772 passed, 105 skipped, 8 errors; the 3 failures + 8 errors are pre-existing on unpatched main with identical test IDs (missing optional depssacrebleu/haystack,test_prechunked_generation,test_experiment)ruff format --checkon the two touched files → already formatted;ruff check src docs tests→All checks passed!pyright src/ragas/dataset_schema.py→0 errors, 0 warnings, 0 informationsReferences
sample()(a different method) — no overlap with this changeSingleMetricAnnotation.stratified_batchesdocstring insrc/ragas/dataset_schema.pyScreenshots/Examples (if applicable)
n/a