From 8b44ae3cb01e05fcf398e0cf028ceb31835e3548 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 15:31:02 +0000 Subject: [PATCH] Studio: revert DeepSeek OCR Image Size knob + move missing-key reset Round 3 of the parallel-reviewer pass surfaced two issues that I had introduced earlier in this PR's follow-ups. - trainer.py: my prior change threaded vision_image_size into the DeepSeek OCR collator's image_size argument. The collator's (image_size, base_size, crop_mode) is a single preset (Tiny / Small / Base / Large / Gundam); changing image_size in isolation desynchronizes the per-crop pixel grid from num_queries downstream and produces wrong token grids on documents larger than the per-crop tile. The fix pins the collator back at the Gundam preset and logs a clear "ignored for DeepSeek OCR" notice when the user has selected a non-default Image Size. - model-defaults.ts + training-config-store.ts: the round 4 fix that reset visionImageSize when a model YAML omitted the key also fired on same-model reloads (ensureModelDefaultsLoaded re-fires on page refresh), wiping a value the user had just selected. The reset is now in setSelectedModel, gated on selectedModel != previousModel, so true model switches still clear stale values while reloads keep the user's selection. --- studio/backend/core/training/trainer.py | 24 ++++++++++--------- .../features/training/lib/model-defaults.ts | 11 ++++----- .../training/stores/training-config-store.ts | 13 +++++++++- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/studio/backend/core/training/trainer.py b/studio/backend/core/training/trainer.py index b20d55aff8..7adc0351c0 100644 --- a/studio/backend/core/training/trainer.py +++ b/studio/backend/core/training/trainer.py @@ -3057,22 +3057,24 @@ class UnslothTrainer: logger.info("Configuring DeepSeek OCR data collator...\n") FastVisionModel.for_training(self.model) - # Honor user-selected Image Size. Keep base_size + crop_mode - # at the Gundam preset (1024 / True) so dynamic cropping of - # large documents still works. - vision_image_size = training_args.get("vision_image_size") - deepseek_image_size = ( - 640 if vision_image_size is None else int(vision_image_size) - ) - if vision_image_size is not None: + # DeepSeek OCR's (image_size, base_size, crop_mode) tuple + # is a single preset (Tiny / Small / Base / Large / Gundam). + # Changing image_size in isolation desynchronizes the per- + # crop pixel grid from num_queries downstream, so the + # user-selected vision_image_size is intentionally ignored + # here. Default to the Gundam preset, which is the + # recommended training configuration. Threading the Image + # Size knob through DeepSeek OCR requires patching + # dynamic_preprocess first. + if training_args.get("vision_image_size") is not None: logger.info( - f"DeepSeek OCR image resize: " - f"{deepseek_image_size} (per-crop tile size)\n" + "Vision image resize ignored for DeepSeek OCR " + "(uses fixed Gundam preset).\n" ) data_collator = DeepSeekOCRDataCollator( tokenizer = self.tokenizer, model = self.model, - image_size = deepseek_image_size, + image_size = 640, base_size = 1024, crop_mode = True, train_on_responses_only = training_args.get( diff --git a/studio/frontend/src/features/training/lib/model-defaults.ts b/studio/frontend/src/features/training/lib/model-defaults.ts index a012dab61a..90dd44acf0 100644 --- a/studio/frontend/src/features/training/lib/model-defaults.ts +++ b/studio/frontend/src/features/training/lib/model-defaults.ts @@ -130,11 +130,10 @@ export function mapBackendModelConfigToTrainingPatch( const randomSeed = toNumber(training?.random_seed); if (randomSeed !== undefined) patch.randomSeed = randomSeed; - // Switching models must reset image size to the model default. Sibling - // training fields would also be reset by their respective branches above, - // but vision_image_size is special: model-default YAMLs in-tree currently - // omit the key, so without an explicit reset a stale 2048 from a previous - // model would silently apply to the new one. + // Only patch visionImageSize when the model config explicitly carries it. + // Resetting a stale value on model SWITCH happens in + // training-config-store.ts setSelectedModel, not here, so that same-model + // defaults reloads do not wipe a value the user just selected. if (Object.hasOwn(training ?? {}, "vision_image_size")) { const raw = training?.vision_image_size; if (raw == null) { @@ -150,8 +149,6 @@ export function mapBackendModelConfigToTrainingPatch( patch.visionImageSize = null; } } - } else { - patch.visionImageSize = null; } const packing = toBoolean(training?.packing); diff --git a/studio/frontend/src/features/training/stores/training-config-store.ts b/studio/frontend/src/features/training/stores/training-config-store.ts index 8d47ad768e..831ce6a40a 100644 --- a/studio/frontend/src/features/training/stores/training-config-store.ts +++ b/studio/frontend/src/features/training/stores/training-config-store.ts @@ -502,7 +502,18 @@ export const useTrainingConfigStore = create()( }, setSelectedModel: (selectedModel) => { const previousModel = get().selectedModel; - set({ selectedModel, modelDefaultsError: null }); + // True model switch resets the image size sentinel so a stale + // value from a previous model does not silently apply to the new + // one. We do this here (not in mapBackendModelConfigToTrainingPatch) + // so same-model defaults reloads do not wipe the user's choice. + const patch: { selectedModel: string | null; modelDefaultsError: null; visionImageSize?: number | null } = { + selectedModel, + modelDefaultsError: null, + }; + if (selectedModel !== previousModel) { + patch.visionImageSize = DEFAULT_HYPERPARAMS.visionImageSize; + } + set(patch); if (!selectedModel) { _modelConfigController?.abort();