Repository navigation
assign_splits's own non-empty-val guarantee doesn't fire when every real task hashes into test #271
Description
Activity
I am working on a focused fix for this. I will first add regression coverage for the all-test hash assignment, preserve hash-assigned test tasks in normal cases, and make any emergency test-to-train/val rebalance explicit in the logs. I will post the PR and validation results here once the change has been reviewed locally.
Opened #276 with the additive shape you proposed.
Confirmed the no-op on
79124b37: 5 tasks atval_fraction=0.10, test_fraction=0.80, seed=42givesCounter({'test': 5}), both guarantees silently no-op.Each guarantee keeps its existing preferred source and only reaches into
testwhen that source is empty, so #235's stability guarantee is untouched on the normal path. Logged throughlogging.getLogger("skillopt_sleep").warningwith the task count, both fractions and the seed.One case beyond what the issue describes: when dream tasks are present they already occupy
train, so the train guarantee is satisfied and only the val guarantee fires — filling val fromteststill has to happen there. Relatedly, topping uptrainnow preferstestover a single-taskval, since taking the only val row would re-empty the gate that was just filled; with no test slice it falls back tovalexactly as before.Full suite: 1501 passed, 12 skipped, 362 subtests. Reverting
mine.pyalone turns 7 of the new assertions red.#272 satisfies the single-batch nonempty-pool and minimum-reassignment checks, including the requested 6,300-case matrix and explicit warnings. A new two-night synthetic integration check found that a task promoted from test to training can silently return to the held-out test set after re-mining with a larger pool; the cycle reports holdout_leaked=false. Please retain the bounded fallback but add durable exposure/split handling, or fail closed when a pristine heldout set cannot be preserved. Keep one coherent solution and leave this issue open pending that longitudinal regression and official CI.
assign_splitsinskillopt_sleep/mine.pysays right in its own comment "Guarantee val (the gate) is non-empty when we have >=2 real tasks", but there's a gap:_promote_oneonly pulls fromtrain(to top upval) or fromval(to top uptrain), never fromtest. If every real task's hash bucket lands in[val_cut, test_cut), bothtrainandvalstart empty, so both guarantee calls have nothing to promote and silently no-op. A nightly batch mines N real tasks, all go totest, andrun_sleep_cyclefinishes withgate_action='reject',edits=0, no error, no warning,holdout_leakeddoesn't flag it either.Not contrived: for a small nightly batch (2-5 tasks is realistic for a solo user's session), any
test_fractionabove roughly 0.5 makes this a matter of when, not if. Confirmed on79124b37: 5 tasks withval_fraction=0.10, test_fraction=0.80all land in test, val and train both come back empty.I think I see why it's shaped this way. #235 replaced the old unconditional
real[-1].split = "val"fallback (append-order-unstable, could demote an already hash-assigned test task) with the current from-train/from-val-only_promote_one, to stop reassigning hash-assigned test tasks per that review. That fixed the instability and reopened this as a side effect.An additive fix, only reach into
testfor the promotion whentrainandvalare both empty, and log it the wayconsolidate.pyalready logsholdout_leaked, would keep the stability guarantee for the normal case and only touch the pathological one. Wanted to check that's the right shape before sending a PR, since it's touching the val/test tradeoff #235 just settled.