diff --git a/studio/backend/core/training/trainer.py b/studio/backend/core/training/trainer.py index 7adc0351c0..b9643cac6a 100644 --- a/studio/backend/core/training/trainer.py +++ b/studio/backend/core/training/trainer.py @@ -3057,15 +3057,9 @@ class UnslothTrainer: logger.info("Configuring DeepSeek OCR data collator...\n") FastVisionModel.for_training(self.model) - # 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. + # DeepSeek OCR's (image_size, base_size, crop_mode) is a + # coupled preset; changing image_size alone desyncs the + # per-crop pixel grid from num_queries. Use Gundam. if training_args.get("vision_image_size") is not None: logger.info( "Vision image resize ignored for DeepSeek OCR " diff --git a/studio/backend/core/training/worker.py b/studio/backend/core/training/worker.py index 1db3f318e8..b5518b65c1 100644 --- a/studio/backend/core/training/worker.py +++ b/studio/backend/core/training/worker.py @@ -965,10 +965,8 @@ def _mlx_vlm_max_resized_size(width: int, height: int, target: int) -> tuple[int largest_side = max(width, height) if largest_side <= target: return width, height - # Mirror UnslothVisionDataCollator's integer formula at - # unsloth_zoo/vision_utils.py so MLX and Torch produce the same pixels. - # Python's round() uses banker's rounding which can disagree by 1px on - # half-pixel cases (e.g. 333x1000 with target 500). + # Integer formula matches unsloth_zoo's collator (Python round() differs + # by 1px on half-pixel cases). max(1, _) avoids zero-side degenerate output. new_w = max(1, (width * target + largest_side // 2) // largest_side) new_h = max(1, (height * target + largest_side // 2) // largest_side) return new_w, new_h @@ -989,9 +987,8 @@ def _resize_mlx_vlm_image(image, resize): if new_size != image.size: resampling = getattr(Image, "Resampling", Image).LANCZOS image = image.resize(new_size, resampling) - # mlx-vlm's internal collator square-resizes PIL images. Return a writable - # ndarray so Studio's max-dimension resize is the final one (like - # trainer.py) and HF processors don't warn on non-writable views. + # Return a writable ndarray so mlx-vlm skips its PIL-path square-resize + # and HF processors don't warn on non-writable views. return np.array(image, copy = True) @@ -1211,9 +1208,7 @@ def _run_mlx_training(event_queue, stop_queue, config): is_vlm = bool(is_dataset_image and getattr(model, "_is_vlm_model", False)) model._is_vlm_model = is_vlm vision_image_size = config.get("vision_image_size") - # Mirror the Torch trainer.py exclusion: DeepSeek OCR's preset is a tuple - # (image_size, base_size, crop_mode), so resizing dataset images outside - # that preset desyncs the token grid. Skip the resize on MLX too. + # DeepSeek OCR uses a coupled preset tuple; skip resize like the Torch path. _model_name_lower = str(config.get("model_name", "")).lower() _is_deepseek_ocr = "deepseek" in _model_name_lower and "ocr" in _model_name_lower if is_vlm and vision_image_size is not None and _is_deepseek_ocr: diff --git a/studio/backend/models/training.py b/studio/backend/models/training.py index 4b3d2a98b9..1246d2debd 100644 --- a/studio/backend/models/training.py +++ b/studio/backend/models/training.py @@ -169,8 +169,7 @@ class TrainingStartRequest(BaseModel): @field_validator("vision_image_size", mode = "before") @classmethod def _check_vision_image_size(cls, v: Any) -> Optional[int]: - # mode="before" runs ahead of Pydantic's int coercion so True/False - # surface as bool (not 1/0) and we can give a precise error. + # mode="before" sees True/False as bool (not 1/0) for a precise error. if v is None: return v if isinstance(v, bool): @@ -182,7 +181,7 @@ class TrainingStartRequest(BaseModel): elif isinstance(v, float) and v.is_integer(): coerced = int(v) else: - # numpy ints and other Integral subclasses (no hard numpy import). + # numpy ints / Integral subclasses, without a hard numpy import. try: import numbers diff --git a/studio/backend/tests/test_mlx_training_worker_config.py b/studio/backend/tests/test_mlx_training_worker_config.py index 438c6cc666..c36363b1ae 100644 --- a/studio/backend/tests/test_mlx_training_worker_config.py +++ b/studio/backend/tests/test_mlx_training_worker_config.py @@ -91,8 +91,6 @@ def test_mlx_vlm_resize_uses_max_dimension_like_torch_trainer(): assert _mlx_vlm_max_resized_size(1000, 1000, 512) == (512, 512) assert _mlx_vlm_max_resized_size(256, 128, 1536) == (256, 128) assert _mlx_vlm_max_resized_size(512, 256, 512) == (512, 256) - # Half-pixel cases must match the Torch collator's integer formula - # (w * size + size_func // 2) // size_func, not Python round() which - # uses banker's rounding. + # Half-pixel cases must match the Torch collator (not banker's round). assert _mlx_vlm_max_resized_size(333, 1000, 500) == (167, 500) assert _mlx_vlm_max_resized_size(1000, 333, 500) == (500, 167) diff --git a/studio/backend/tests/test_studio_train_validation.py b/studio/backend/tests/test_studio_train_validation.py index 56edefd23f..2be87dc812 100644 --- a/studio/backend/tests/test_studio_train_validation.py +++ b/studio/backend/tests/test_studio_train_validation.py @@ -87,8 +87,7 @@ class TestVisionImageSizeCap: @pytest.mark.parametrize("value", [True, False]) def test_bool_error_says_integer_not_range(self, value): - # Regression guard: pre-fix the bool was coerced to 1/0 and the - # message read "must be in [256, 2048] (got 1)", which was confusing. + # Regression guard: bools must say "integer or null", not "in [256, 2048]". with pytest.raises(ValidationError) as exc: _check_field("vision_image_size", value) assert "integer or null" in str(exc.value) diff --git a/studio/frontend/src/features/studio/sections/params-section.tsx b/studio/frontend/src/features/studio/sections/params-section.tsx index 799ae4e059..22a5f4bfd7 100644 --- a/studio/frontend/src/features/studio/sections/params-section.tsx +++ b/studio/frontend/src/features/studio/sections/params-section.tsx @@ -133,9 +133,7 @@ export function ParamsSection(): ReactElement { const isCpt = store.trainingMethod === "cpt"; const isRawText = isRawTextDatasetFormat(store.datasetFormat); const showVisionLora = store.isVisionModel && store.isDatasetImage === true; - // DeepSeek OCR's preset is a coupled tuple, so the backend ignores any - // user-selected image size for it. Hide the control rather than offer a - // setting that silently has no effect. + // DeepSeek OCR uses a coupled preset; backend ignores user image size. const _selectedModelLower = (store.selectedModel ?? "").toLowerCase(); const isDeepseekOcr = _selectedModelLower.includes("deepseek") && diff --git a/studio/frontend/src/features/studio/sections/training-section.tsx b/studio/frontend/src/features/studio/sections/training-section.tsx index 8970da31a3..270831475b 100644 --- a/studio/frontend/src/features/studio/sections/training-section.tsx +++ b/studio/frontend/src/features/studio/sections/training-section.tsx @@ -80,16 +80,12 @@ export function TrainingSection() { }; const handleSaveConfig = () => { - // Save vision fields for any vision-capable model unless we have positive - // confirmation the dataset is text-only. isDatasetImage is undetermined - // (null) before a dataset check completes, after dataset edits, and on - // import; treating that as "don't save" would silently drop the user's - // vision_image_size choice in those windows. + // isDatasetImage is null during dataset checks; treat that as "save it" + // so an in-flight check doesn't silently drop the user's choice. const includeVisionFields = store.isVisionModel && store.isDatasetImage !== false; - // DeepSeek OCR ignores vision_image_size at training time (mappers.ts - // sends null), so do not emit it to YAML either; otherwise a stale - // value could later apply to a non-DeepSeek vision model. + // DeepSeek OCR ignores vision_image_size; don't emit it to YAML either, + // or a later import on a non-DeepSeek model would activate the stale value. const selectedModelLower = (store.selectedModel ?? "").toLowerCase(); const isDeepseekOcr = selectedModelLower.includes("deepseek") && diff --git a/studio/frontend/src/features/training/lib/model-defaults.ts b/studio/frontend/src/features/training/lib/model-defaults.ts index 90dd44acf0..7c6149310a 100644 --- a/studio/frontend/src/features/training/lib/model-defaults.ts +++ b/studio/frontend/src/features/training/lib/model-defaults.ts @@ -130,18 +130,14 @@ export function mapBackendModelConfigToTrainingPatch( const randomSeed = toNumber(training?.random_seed); if (randomSeed !== undefined) patch.randomSeed = randomSeed; - // 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. + // Only patch when the config carries the key; model-switch reset lives in + // setSelectedModel so same-model reloads don't wipe a user's choice. if (Object.hasOwn(training ?? {}, "vision_image_size")) { const raw = training?.vision_image_size; if (raw == null) { patch.visionImageSize = null; } else { - // Mirror the backend validator at studio/backend/models/training.py:169. - // Anything not an integer in [256, 2048] is dropped so the store and UI - // never show a value the backend would reject. + // Drop anything the backend validator would reject. const n = toNumber(raw); if (n !== undefined && Number.isInteger(n) && n >= 256 && n <= 2048) { patch.visionImageSize = n; diff --git a/studio/frontend/src/features/training/lib/yaml-config.ts b/studio/frontend/src/features/training/lib/yaml-config.ts index 301bbb437c..1deabf3547 100644 --- a/studio/frontend/src/features/training/lib/yaml-config.ts +++ b/studio/frontend/src/features/training/lib/yaml-config.ts @@ -27,15 +27,10 @@ export function parseYamlConfig(text: string): BackendModelConfig { console.warn("Ignored unknown YAML keys:", unknownKeys.join(", ")); } - // YAML import means "use this config as authoritative". An absent - // vision_image_size should reset the in-memory value to Default, not - // preserve a stale one. Same-model defaults reloads (which also flow - // through the model-config mapper) skip the reset via Object.hasOwn - // in model-defaults.ts; here we forge the key so import always wins. - // This also covers configs where the training section is missing, - // null, an array, or any non-mapping scalar - in all such cases the - // mapper would otherwise emit no vision patch and the previously - // selected image size would silently persist. + // File import is authoritative: forge vision_image_size = null when the + // training section is missing, malformed, or missing the key, so a stale + // store value cannot survive an import. (Same-model defaults reloads + // preserve user choice via Object.hasOwn in model-defaults.ts.) const rawTraining = raw.training; const isPlainTrainingObject = rawTraining != null && 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 831ce6a40a..137669f8b8 100644 --- a/studio/frontend/src/features/training/stores/training-config-store.ts +++ b/studio/frontend/src/features/training/stores/training-config-store.ts @@ -390,9 +390,7 @@ export const useTrainingConfigStore = create()( error instanceof Error ? error.message : "Failed to load model defaults", - // Defaults load failed, so we cannot map the new model's - // vision_image_size. Reset to the global default so a stale - // value from a prior model never silently applies. + // Defaults load failed; reset so no prior model's value lingers. visionImageSize: DEFAULT_HYPERPARAMS.visionImageSize, }); @@ -502,10 +500,8 @@ export const useTrainingConfigStore = create()( }, setSelectedModel: (selectedModel) => { const previousModel = get().selectedModel; - // 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. + // Reset vision_image_size on a true switch only; same-model reloads + // go through the mapper, which preserves the user's choice. const patch: { selectedModel: string | null; modelDefaultsError: null; visionImageSize?: number | null } = { selectedModel, modelDefaultsError: null,