Skip to content

fix(instance-preferences): clear error on recipe trainers; no serializer warnings on submit - #6280

Open
deeppcs wants to merge 2 commits into
aws:masterfrom
deeppcs:ip-recipe-guard
Open

deeppcs wants to merge 2 commits into
aws:masterfrom
deeppcs:ip-recipe-guard

Conversation

@deeppcs

@deeppcs deeppcs commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Description

Two follow-ups to Instance Preferences (#6249), found while verifying the 3.22.0 release end to end against the service.

Recipe trainers reject instance_preferences with a clear error. SFTTrainer, DPOTrainer and RLVRTrainer accept a Compute for serverful training, and Compute now carries instance_preferences. Recipes are rendered for one instance type — the device class selects the image and launcher, and the type is validated against the model's allowed-type list — so a service-chosen type cannot apply and the feature is documented as unsupported there. Today that surfaces indirectly: with no instance_type set, the allowed-type check reports Instance type 'None' is not supported before ModelTrainer.from_recipe can raise its explicit message. _train_serverful_smtj now rejects a preference list up front, before the recipe fetch, with the same wording from_recipe uses.

Five existing tests model Compute as a bare MagicMock, whose auto-created instance_preferences attribute is truthy; they now pin it to None, as a real Compute without preferences reports (the same treatment #6270 applied for training_plan_arn).

No pydantic serializer warnings on submit. Compute's validator converted Unassigned() to None on its own fields but not on the nested InstancePreference objects, so every submit with a preference list emitted one PydanticSerializationUnexpectedValue warning per element (for the unset training_plan_arns list) when _to_resource_config() called model_dump(). The request payload was already correct; the noise was the only effect. Both Compute classes now normalise each preference the same way; the serialized ResourceConfig is unchanged.

Testing

  • Unit: 195 sagemaker-core (compute configs, processing, service model, shapes extractor), 156 sagemaker-train (model_trainer, defaults, base trainer serverful/compute, serverful recipe validation). No new failures across the SFT/DPO/RLVR trainer suites relative to master.
  • The recipe guard was reproduced before/after against an SFTTrainer on the serverful path: the message changes from Instance type 'None' is not supported. Allowed values: [...] to Training recipes do not support instance_preferences. ...; the new test also asserts the recipe is never fetched.
  • The serializer test runs _to_resource_config() with warnings turned into errors and fails without the fix; the serialized payload was compared byte-for-byte before and after.
  • With the released 3.22.0 packages, seven training and two processing preference-list combinations (uniform and per-preference counts, two to four candidates, mixed generations, MaxPendingTimeInSeconds=1800) submitted, echoed and completed on a resolved winner against the service in us-west-2; the warning fixed here appeared once per preference on every one of those submits.

… with a clear error

Recipe-based trainers (SFTTrainer, DPOTrainer, RLVRTrainer) accept a
Compute for serverful training, and Compute now carries
instance_preferences. Recipes are rendered for one instance type: the
device class selects the image and launcher, and the type is validated
against the model's allowed-type list. A service-chosen type cannot apply,
so instance_preferences is unsupported on this path, as documented.

Today the unsupported case surfaces indirectly. With no instance_type
set, the allowed-type check reports "Instance type 'None' is not
supported" before ModelTrainer.from_recipe can raise its explicit
message. _train_serverful_smtj now rejects a preference list up front,
before the recipe fetch, with the same wording from_recipe uses.

Two existing tests modelled Compute as a bare MagicMock, whose
auto-created instance_preferences attribute is truthy; they now pin it to
None, as a real Compute without preferences reports.
…t preference fields

Compute's validator converts Unassigned() to None on its own fields but
not on the nested InstancePreference objects, so every submit with a
preference list emitted one PydanticSerializationUnexpectedValue warning
per element (for the unset training_plan_arns list) when
_to_resource_config() called model_dump(). The request payload was
already correct; the noise was the only effect.

Both Compute classes now normalise each preference the same way. The
serialized ResourceConfig is unchanged. A test asserts
_to_resource_config() completes with warnings turned into errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant