Skip to content

Commit 3f87292

Browse files
author
rsareddy0329
committed
test(train): assert override flows into resolved recipe, not serverless HP
The scoped CodeBuild run surfaced that asserting override flow through the serverless-filtered hyperparameters is wrong for renamed-leaf keys (e.g. dataset_max_len -> training_config.data.max_length): the serverless path in _apply_recipe_to_hyperparameters filters flattened recipe leaf names against user-provided spec names, so renamed-leaf overrides are dropped. Assert override application against the resolved recipe (the resolver's contract) instead, keeping the test recipe-proof and leaf-name-agnostic. The serverless drop of renamed-leaf overrides is a separate SDK issue to be raised with the recipe owners.
1 parent b784036 commit 3f87292

1 file changed

Lines changed: 10 additions & 8 deletions

File tree

‎sagemaker-train/tests/integ/train/test_recipe_override_integration.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -863,22 +863,24 @@ def test_sft_llama_serverless_only_user_override_keys_applied(self, sagemaker_se
863863
logger.info(f"llama SFT — User-override-only HP keys: {len(result_hp)}")
864864
logger.info(f"llama SFT — Full recipe HP keys: {len(full_hp)}")
865865

866-
# Override VALUES must flow through (asserted by value, robust to the
867-
# recipe's leaf renames e.g. learning_rate -> optim.lr).
868-
result_values = set(result_hp.values())
866+
# Override VALUES flow into the resolved recipe (resolver correctness).
867+
# Asserted against the resolved recipe (not the serverless-filtered
868+
# hyperparameters) so it is robust to recipe leaf renames, e.g.
869+
# dataset_max_len -> training_config.data.max_length.
870+
resolved_recipe = sft_trainer.get_resolved_recipe()
871+
flat_recipe = flatten_resolved_recipe(resolved_recipe)
872+
resolved_values = {v for v in flat_recipe.values() if v is not None}
869873
for key, val in override_values.items():
870874
assert (
871-
str(val) in result_values
872-
), f"override {key}={val} did not flow into serverless hyperparameters"
875+
val in resolved_values
876+
), f"override {key}={val} did not flow into the resolved recipe"
873877

874878
# All values are strings (SageMaker API requirement).
875879
for k, v in result_hp.items():
876880
assert isinstance(v, str), f"Key '{k}' non-string: {type(v).__name__}"
877881

878882
# Dynamically compute recipe keys NOT in the override spec, then assert
879-
# they are excluded from the serverless result but present in the full one.
880-
resolved_recipe = sft_trainer.get_resolved_recipe()
881-
flat_recipe = flatten_resolved_recipe(resolved_recipe)
883+
# they are excluded from the serverless result (reusing flat_recipe above).
882884
all_recipe_keys = {k for k, v in flat_recipe.items() if v is not None}
883885
override_spec_keys = set(sft_trainer.hyperparameters._specs.keys())
884886
user_leaf_keys = set(flatten_resolved_recipe({"training_config": override_values}).keys())

0 commit comments

Comments
 (0)