From f3711f00e2288becdb9ad59b74c79cdacebac7ea Mon Sep 17 00:00:00 2001 From: rockdu Date: Mon, 10 Aug 2026 14:30:01 -0700 Subject: [PATCH] fix(args): stop a custom TrainPipelineConfig from clearing the model family The model family is a separate axis from the config class. It picks the config class when no path is given, and it independently picks the encoder module -- encoder_hub.get_encoder(args.diffusion_model_family), read by the SFT validation block and by SftEncodeActor. The --train-pipeline-config-path branch cleared it to None on the grounds that "explicit config path IS the identity". That is the identity of the config class, not of the family, so a custom config could not run SFT at all. #142 then rejected the two flags together, walling off the only combination that could have supplied the family. Drop both: the path names the config class, the family stays whatever was passed. Normalization moves ahead of the branch so it applies on either path. No new validation. The family stays optional on purpose -- encoder and other per-family pieces may become customizable in their own right, and a check here would then be forbidding a legitimate run. Whoever needs the family fails at the lookup that needs it; get_encoder's message now names the flag to pass so that failure is actionable. The six recipes' command lines are unchanged (7 captured variants identical); none of them passes --train-pipeline-config-path. Co-Authored-By: Claude Opus 5 --- miles/rollout/encoder_hub/__init__.py | 5 ++++- miles/utils/arguments.py | 15 +++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/miles/rollout/encoder_hub/__init__.py b/miles/rollout/encoder_hub/__init__.py index 4e04b245..98b9e032 100644 --- a/miles/rollout/encoder_hub/__init__.py +++ b/miles/rollout/encoder_hub/__init__.py @@ -14,4 +14,7 @@ def get_encoder(family: str | None): from miles.rollout.encoder_hub import wan2_2 return wan2_2 - raise ValueError(f"no encoder_hub entry for model family {family!r}") + raise ValueError( + f"no encoder_hub entry for model family {family!r}; set --diffusion-model-family to one " + "that has an entry, or add a module here for it" + ) diff --git a/miles/utils/arguments.py b/miles/utils/arguments.py index 05b2e30e..725625e5 100644 --- a/miles/utils/arguments.py +++ b/miles/utils/arguments.py @@ -1472,12 +1472,15 @@ def miles_validate_args(args): from miles.utils.misc import load_function + if args.diffusion_model_family is not None: + # Downstream lookups compare this exactly (encoder_hub.get_encoder), so normalize once + # here rather than at every reader. + args.diffusion_model_family = args.diffusion_model_family.strip().lower() + if args.train_pipeline_config_path is not None: - if args.diffusion_model_family is not None: - raise ValueError("--train-pipeline-config-path and --diffusion-model-family both name a config; pass one.") - # Explicit config path IS the identity (custom classes never need registering). + # The path names the config class; the family stays whatever was passed, since it is a + # separate axis (encoder_hub) and may be unset for a model that customizes that too. cfg_cls = load_function(args.train_pipeline_config_path) - args.diffusion_model_family = None else: from miles.backends.fsdp_utils.configs.train_pipeline_config import ( get_train_pipeline_config_cls, @@ -1486,10 +1489,6 @@ def miles_validate_args(args): if args.diffusion_model_family is None: args.diffusion_model_family = resolve_diffusion_model_family(args.hf_checkpoint) - else: - # Downstream lookups compare this exactly (encoder_hub.get_encoder), so normalize - # here rather than at every reader. - args.diffusion_model_family = args.diffusion_model_family.strip().lower() cfg_cls = get_train_pipeline_config_cls(args.diffusion_model_family) args.train_pipeline_config_path = f"{cfg_cls.__module__}.{cfg_cls.__qualname__}" if args.model_backend_path is None: