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