Make the random-control source explicit in the validate step - #445
Open
luciaquirke wants to merge 1 commit into
Open
Make the random-control source explicit in the validate step#445luciaquirke wants to merge 1 commit into
luciaquirke wants to merge 1 commit into
Conversation
Today the control source is chosen by implicit precedence: a non-empty retrained_dir loads a bank, otherwise num_subsets > 0 retrains in-process, otherwise controls are skipped. Nothing in a config states which will happen, so a config that retrains its controls is indistinguishable from one that loads them. That is worth surfacing because controls are retrained PER PROCESS. The controls are N trained models for the whole run, and scoring them against every query is only forward passes -- so splitting a filter into S shards and letting each retrain its own turns N controls into N*S full trainings. On a 128k-document row sharded 10 ways that was 30 trainings where 3 suffice, roughly 33 GPU-pair-hours, and nothing in the output made it obvious. Adds ValidationConfig.controls: auto | load | retrain | skip. auto (default) the existing precedence, so no behaviour changes load require retrained_dir; error rather than silently retraining retrain require num_subsets > 0; ignore any bank skip no random baseline The retrain path now also notes that its controls are per-process and points at the shared alternative, so a sharded run says so in its own log rather than leaving the cost to be inferred.
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.
The control source is currently chosen by implicit precedence: a non-empty
retrained_dirloads a bank, otherwisenum_subsets > 0retrains in-process, otherwise controls are skipped. Nothing in a config states which will happen, so a config that retrains its controls looks identical to one that loads them.That is worth surfacing because controls are retrained per process. The controls are N trained models for the whole run, and scoring them against every query is only forward passes — so splitting a filter into S shards and letting each retrain its own turns N controls into N*S full trainings.
We hit this on a 128k-document row sharded 10 ways: 30 trainings where 3 suffice, roughly 33 GPU-pair-hours, and nothing in the output made it obvious. The sharding script printed the total and it was read past; the config itself gave no signal at all.
Change
Adds
ValidationConfig.controls: auto | load | retrain | skip.autoloadretrained_dir; error rather than silently retrainingretrainnum_subsets > 0; ignore any bankskipThe retrain path also notes that its controls are per-process and points at the shared alternative, so a sharded run says so in its own log rather than leaving the cost to be inferred.
Backwards compatible:
autois the default and reproduces current behaviour exactly. Existing configs are unaffected.