fix(sleep): keep the gate non-empty when every task hashes into test - #276
fix(sleep): keep the gate non-empty when every task hashes into test#276hylin (linhongyu510) wants to merge 1 commit into
Conversation
assign_splits promises "Guarantee val (the gate) is non-empty when we have >=2 real tasks", but _promote_one only pulls from train (to top up val) or from val (to top up train), never from test. When every real task's hash bucket lands in [val_cut, test_cut) both train and val start empty, so both guarantee calls have nothing to promote and silently no-op. run_sleep_cycle then finishes with gate_action='reject' and edits=0: no error, no warning, and holdout_leaked does not flag it either. For a small nightly batch any test_fraction above roughly 0.5 makes this a matter of when, not if. Fall back to test for the promotion only when the preferred source is empty, so the stability guarantee microsoft#235 settled is untouched in the normal case and hash-assigned test tasks are only reassigned in the degenerate one. Topping up train now prefers test over a single-task val, so filling train cannot re-empty the gate it just filled. Borrowing spends a held-out task, so it is logged the way consolidate.py already logs holdout_leaked. Tests cover the degenerate split for 2..10 real tasks, the warning, dream tasks in train not masking an empty gate, the normal path not borrowing, and the single-real-task guard.
|
Closing as a duplicate — I missed #272, which was opened three days earlier for the same issue, touches the same two files, and lands the same fallback (prefer the non-test pool, reach into test only when both required pools cannot otherwise be filled, and warn when test coverage is spent). Apologies for the noise; #272 should be the one reviewed. One detail from my run that may be worth folding into #272 if it is not already covered: when dream tasks are present they occupy |
|
Duplicate of #272. |
Fixes #271.
The gap
assign_splitsinskillopt_sleep/mine.pysays right in its own comment"Guarantee val (the gate) is non-empty when we have >=2 real tasks", but
_promote_oneonly pulls fromtrain(to top upval) or fromval(to topup
train), never fromtest. If every real task's hash bucket lands in[val_cut, test_cut), bothtrainandvalstart empty, so both guaranteecalls have nothing to promote and silently no-op.
run_sleep_cyclethen finishes withgate_action='reject',edits=0, noerror, no warning, and
holdout_leakeddoes not flag it either. The nightlooks like it ran and learned nothing, with no signal saying why.
As #271 notes, this is not contrived: for a small nightly batch (2-5 tasks is
realistic for a solo user), any
test_fractionabove roughly 0.5 makes it amatter of when, not if.
Reproduction on
79124b37Before:
Counter({'test': 5})— val and train both empty, no warning.After:
Counter({'test': 3, 'val': 1, 'train': 1})plus aWARNINGnaming thefractions and seed.
The change
Two small, additive adjustments in
assign_splits:_promote_onenow returns whether it promoted anything, so a no-op isdistinguishable from a promotion.
testwhen that source is empty.#235's stability guarantee is untouchedin the normal case — hash-assigned
testtasks are reassigned only in thedegenerate case where the alternative is a dead cycle.
Topping up
trainpreferstestover a single-taskval, because takingthe only val row would re-empty the gate that was just filled. When there is no
test slice at all, it still falls back to
valexactly as before.Borrowing does spend a held-out task, so it is logged via
logging.getLogger("skillopt_sleep").warning(...)— the module's existingconvention — naming the task count, both fractions and the seed, and pointing
at
test_fractionas the knob to lower.This is the shape #271 proposed. I also hit one case the issue does not
mention: when dream tasks are present they occupy
train, so the secondguarantee is already satisfied and only the val guarantee fires. Filling val
from
teststill has to happen there, andvalstays real-only.Tests
Added to
Pass1ApproachBAssignSplitsInvariants, next to the existing #235stability test:
test_val_guaranteed_when_every_real_task_hashes_into_test— subtests over2..10 real tasks
test_degenerate_split_warns_that_test_was_spenttest_dream_train_does_not_mask_an_empty_gate— val stays real-only; dreamstays train
test_normal_split_does_not_borrow_from_test— asserts no logger is evenconstructed on the healthy path
test_single_real_task_is_left_alone— the>=2guard still appliesReverting
mine.pyalone turns 7 of these red, so they pin the behavior ratherthan restate it.
Verification
python -m pytest tests/— 1501 passed, 12 skipped, 362 subtests passedpython -m ruff check skillopt_sleep/mine.py— cleantests/test_split_hardening_2x3.pyhas one pre-existing ruffI001import-orderfinding on
main; I left it alone to keep this diff minimal.