microsoft / microsoft/SkillOpt

assign_splits's own non-empty-val guarantee doesn't fire when every real task hashes into test

Open
#271 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
17.3k
Forks
1.6k
Avg merge
2d 7h
Merged PRs (30d)
17

Description

assign_splits in skillopt_sleep/mine.py says right in its own comment "Guarantee val (the gate) is non-empty when we have >=2 real tasks", but there's a gap: _promote_one only pulls from train (to top up val) or from val (to top up train), never from test. If 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. A nightly batch mines N real tasks, all go to test, and run_sleep_cycle finishes with gate_action='reject', edits=0, no error, no warning, holdout_leaked doesn't flag it either.

Not contrived: for a small nightly batch (2-5 tasks is realistic for a solo user's session), any test_fraction above roughly 0.5 makes this a matter of when, not if. Confirmed on 79124b37: 5 tasks with val_fraction=0.10, test_fraction=0.80 all 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 test for the promotion when train and val are both empty, and log it the way consolidate.py already logs holdout_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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in skillopt_sleep/mine.py at assign_splits and _promote_one, reproducing the issue with the cited val_fraction=0.10 and test_fraction=0.80 case. Read consolidate.py's holdout_leaked logging for the expected warning style. Done means a batch with empty train and val receives the intended non-empty-val handling without changing normal hash assignments, and the pathological case is observable.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.