Skip to content

Commit c3397ba

Browse files
admivsnmohamedzeidan2021Mohamed Zeidan
authored
Fix incorrect model_trainer method call to load hyperparameters from file (#5771)
* Fix incorrect method name to load hyperparameters from file file * test(train): cover from_recipe Nova hyperparameters file path Regression tests for the AttributeError in #5770: from_recipe() called a non-existent _validate_and_load_hyperparameters_file when hyperparameters was a path string. Cover JSON and YAML files, the dict path, and a missing file, all of which fail with AttributeError before the one-line fix. --------- Co-authored-by: Mohamed Zeidan <81834882+mohamedzeidan2021@users.noreply.github.com> Co-authored-by: Mohamed Zeidan <zeidmo@amazon.com>
1 parent 29968ef commit c3397ba

2 files changed

Lines changed: 104 additions & 1 deletion

File tree

‎sagemaker-train/src/sagemaker/train/model_trainer.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1561,7 +1561,7 @@ def from_recipe(
15611561
)
15621562
if is_nova:
15631563
if hyperparameters and isinstance(hyperparameters, str):
1564-
hyperparameters = cls._validate_and_load_hyperparameters_file(hyperparameters)
1564+
hyperparameters = cls._validate_and_fetch_hyperparameters_file(hyperparameters)
15651565
model_trainer_args["hyperparameters"].update(hyperparameters)
15661566
elif hyperparameters and isinstance(hyperparameters, dict):
15671567
model_trainer_args["hyperparameters"].update(hyperparameters)

‎sagemaker-train/tests/unit/train/test_model_trainer.py‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,6 +1860,109 @@ def test_nova_recipe_with_distillation(modules_session):
18601860
os.unlink(recipe.name)
18611861

18621862

1863+
@pytest.mark.parametrize(
1864+
"suffix, dump",
1865+
[(".json", json.dumps), (".yaml", yaml.dump)],
1866+
)
1867+
def test_nova_recipe_with_hyperparameters_file(suffix, dump, modules_session):
1868+
"""Hyperparameters passed as a file path are loaded and merged into the recipe's."""
1869+
recipe_data = {
1870+
"run": {
1871+
"name": "dummy-model",
1872+
"model_type": "amazon.nova",
1873+
"model_name_or_path": "dummy-model",
1874+
}
1875+
}
1876+
hyperparameters = {"param1": "value1", "param2": 2}
1877+
1878+
with NamedTemporaryFile(suffix=".yaml", delete=False) as recipe:
1879+
with open(recipe.name, "w") as file:
1880+
yaml.dump(recipe_data, file)
1881+
1882+
with NamedTemporaryFile(suffix=suffix, delete=False, mode="w") as hp_file:
1883+
hp_file.write(dump(hyperparameters))
1884+
1885+
try:
1886+
trainer = ModelTrainer.from_recipe(
1887+
training_recipe=recipe.name,
1888+
role=DEFAULT_ROLE,
1889+
sagemaker_session=modules_session,
1890+
compute=DEFAULT_COMPUTE_CONFIG,
1891+
training_image=DEFAULT_IMAGE,
1892+
hyperparameters=hp_file.name,
1893+
)
1894+
1895+
assert trainer._is_nova_recipe
1896+
assert trainer.hyperparameters == {
1897+
"base_model": "dummy-model",
1898+
"param1": "value1",
1899+
"param2": 2,
1900+
}
1901+
finally:
1902+
os.unlink(hp_file.name)
1903+
os.unlink(recipe.name)
1904+
1905+
1906+
def test_nova_recipe_with_hyperparameters_dict(modules_session):
1907+
"""Hyperparameters passed as a dict are merged into the recipe's."""
1908+
recipe_data = {
1909+
"run": {
1910+
"name": "dummy-model",
1911+
"model_type": "amazon.nova",
1912+
"model_name_or_path": "dummy-model",
1913+
}
1914+
}
1915+
1916+
with NamedTemporaryFile(suffix=".yaml", delete=False) as recipe:
1917+
with open(recipe.name, "w") as file:
1918+
yaml.dump(recipe_data, file)
1919+
1920+
try:
1921+
trainer = ModelTrainer.from_recipe(
1922+
training_recipe=recipe.name,
1923+
role=DEFAULT_ROLE,
1924+
sagemaker_session=modules_session,
1925+
compute=DEFAULT_COMPUTE_CONFIG,
1926+
training_image=DEFAULT_IMAGE,
1927+
hyperparameters={"param1": "value1"},
1928+
)
1929+
1930+
assert trainer.hyperparameters == {
1931+
"base_model": "dummy-model",
1932+
"param1": "value1",
1933+
}
1934+
finally:
1935+
os.unlink(recipe.name)
1936+
1937+
1938+
def test_nova_recipe_with_missing_hyperparameters_file(modules_session):
1939+
"""A hyperparameters path that does not exist raises a ValueError."""
1940+
recipe_data = {
1941+
"run": {
1942+
"name": "dummy-model",
1943+
"model_type": "amazon.nova",
1944+
"model_name_or_path": "dummy-model",
1945+
}
1946+
}
1947+
1948+
with NamedTemporaryFile(suffix=".yaml", delete=False) as recipe:
1949+
with open(recipe.name, "w") as file:
1950+
yaml.dump(recipe_data, file)
1951+
1952+
try:
1953+
with pytest.raises(ValueError, match="Hyperparameters file not found"):
1954+
ModelTrainer.from_recipe(
1955+
training_recipe=recipe.name,
1956+
role=DEFAULT_ROLE,
1957+
sagemaker_session=modules_session,
1958+
compute=DEFAULT_COMPUTE_CONFIG,
1959+
training_image=DEFAULT_IMAGE,
1960+
hyperparameters="nonexistent.json",
1961+
)
1962+
finally:
1963+
os.unlink(recipe.name)
1964+
1965+
18631966
def test_nova_recipe_with_model_package_arn(modules_session):
18641967
"""Test that MP ARN in model_name_or_path routes to ModelPackageConfig."""
18651968
mp_arn = "arn:aws:sagemaker:us-east-1:123456789012:model-package/my-mpg/1"

0 commit comments

Comments
 (0)