Fix for 'upload_to_hf_hub()' path mismatch with 'save()'

ludwig/api.py

@@ -110,6 +110,7 @@
 from ludwig.utils.torch_utils import DEVICE
 from ludwig.utils.trainer_utils import get_training_report
 from ludwig.utils.types import DataFrame, TorchDevice
+from ludwig.utils.upload_utils import HuggingFaceHub
 
 logger = logging.getLogger(__name__)
 
@@ -1938,39 +1939,53 @@ def upload_to_hf_hub(
 
         # Inputs
 
-        :param repo_id (`str`):
+        :param repo_id: (`str`)
             A namespace (user or an organization) and a repo name separated
             by a `/`.
-        :param model_path (`str`):
-            The path of the saved model. This is the top level directory where
-            the models weights as well as other associated training artifacts
-            are saved.
-        :param private (`bool`, *optional*, defaults to `False`):
+        :param model_path: (`str`)
+            The path of the saved model. This is either (a) the directory where
+            the 'models_weights' folder and the 'model_hyperparameters.json' file
+            are stored, or (b) the parent of that folder.
+        :param private: (`bool`, *optional*, defaults to `False`)
             Whether the model repo should be private.
-        :param repo_type (`str`, *optional*):
+        :param repo_type: (`str`, *optional*)
             Set to `"dataset"` or `"space"` if uploading to a dataset or
             space, `None` or `"model"` if uploading to a model. Default is
             `None`.
-        :param commit_message (`str`, *optional*):
+        :param commit_message: (`str`, *optional*)
             The summary / title / first line of the generated commit. Defaults to:
             `f"Upload {path_in_repo} with huggingface_hub"`
-        :param commit_description (`str` *optional*):
+        :param commit_description: (`str` *optional*)
             The description of the generated commit
 
         # Returns
 
         :return: (bool) True for success, False for failure.
         """
+        if os.path.exists(os.path.join(model_path, "model", "model_weights")) and os.path.exists(
+            os.path.join(model_path, "model", MODEL_HYPERPARAMETERS_FILE_NAME)
+        ):
+            model_path_is_experiment_path = True
+        elif os.path.exists(os.path.join(model_path, "model_weights")) and os.path.exists(
+            os.path.join(model_path, MODEL_HYPERPARAMETERS_FILE_NAME)
+        ):
+            model_path_is_experiment_path = False
+        else:
+            raise ValueError(
+                f"Can't find 'model_weights' and '{MODEL_HYPERPARAMETERS_FILE_NAME}' either at "
+                f"'{model_path}' or at '{model_path}/model'"
+            )
         model_service = get_upload_registry()["hf_hub"]
-        hub = model_service()
+        hub: HuggingFaceHub = model_service()
         hub.login()
-        upload_status = hub.upload(
+        upload_status: bool = hub.upload(
             repo_id=repo_id,
             model_path=model_path,
             repo_type=repo_type,
             private=private,
             commit_message=commit_message,
             commit_description=commit_description,
+            model_path_is_experiment_path=model_path_is_experiment_path,
         )
         return upload_status
 

ludwig/utils/upload_utils.py

@@ -43,6 +43,7 @@ def upload(
         commit_description: str | None = None,
         dataset_file: str | None = None,
         dataset_name: str | None = None,
+        model_path_is_experiment_path: bool = True,
     ) -> bool:
         """Abstract method to upload trained model artifacts to the target repository.
 
@@ -67,6 +68,7 @@ def _validate_upload_parameters(
         private: bool | None = False,
         commit_message: str | None = None,
         commit_description: str | None = None,
+        model_path_is_experiment_path: bool = True,
     ):
         """Validate parameters before uploading trained model artifacts.
 
@@ -87,6 +89,8 @@ def _validate_upload_parameters(
             commit_description (str, optional): A description of the commit when uploading to version control
                 systems. Not used in the base class, but subclasses may use it for specific repository
                 implementations. Defaults to None.
+            model_path_is_experiment_path (bool, optional): Whether the 'model_path' argument points to the
+                experiment folder
 
         Raises:
             FileNotFoundError: If the model_path does not exist.
@@ -99,7 +103,10 @@ def _validate_upload_parameters(
             raise FileNotFoundError(f"The path '{model_path}' does not exist.")
 
         # Make sure the model is actually trained
-        trained_model_artifacts_path = os.path.join(model_path, "model", "model_weights")
+        if model_path_is_experiment_path:
+            trained_model_artifacts_path = os.path.join(model_path, "model", "model_weights")
+        else:
+            trained_model_artifacts_path = os.path.join(model_path, "model_weights")
         if not os.path.exists(trained_model_artifacts_path):
             raise Exception(
                 f"Model artifacts not found at {trained_model_artifacts_path}. "
@@ -143,6 +150,7 @@ def _validate_upload_parameters(
         private: bool | None = False,
         commit_message: str | None = None,
         commit_description: str | None = None,
+        model_path_is_experiment_path: bool = True,
     ):
         """Validate parameters before uploading trained model artifacts.
 
@@ -165,6 +173,8 @@ def _validate_upload_parameters(
             commit_description (str, optional): A description of the commit when uploading to version control
                 systems. Not used in the base class, but subclasses may use it for specific repository
                 implementations. Defaults to None.
+            model_path_is_experiment_path (bool, optional): Whether the 'model_path' argument points to the
+                experiment folder
 
         Raises:
             ValueError: If the repo_id does not have both a namespace and a repo name separated by a '/'.
@@ -183,9 +193,13 @@ def _validate_upload_parameters(
             private,
             commit_message,
             commit_description,
+            model_path_is_experiment_path,
         )
 
-        trained_model_artifacts_path = os.path.join(model_path, "model", "model_weights")
+        if model_path_is_experiment_path:
+            trained_model_artifacts_path = os.path.join(model_path, "model", "model_weights")
+        else:
+            trained_model_artifacts_path = os.path.join(model_path, "model_weights")
         """
         Make sure the model's saved artifacts either contain:
         1. pytorch_model.bin -> regular model training, such as ECD or for LLMs
@@ -207,7 +221,10 @@ def _validate_upload_parameters(
                 "either be saved as `pytorch_model.bin` for regular model training, or have `adapter_model.bin`"
                 "or `adapter_model.safetensors` if using parameter efficient fine-tuning methods like LoRA."
             )
-        model_hyperparameters_path: str = os.path.join(model_path, "model")
+        if model_path_is_experiment_path:
+            model_hyperparameters_path: str = os.path.join(model_path, "model")
+        else:
+            model_hyperparameters_path: str = model_path
         if MODEL_HYPERPARAMETERS_FILE_NAME not in os.listdir(model_hyperparameters_path):
             raise ValueError(f"Can't find '{MODEL_HYPERPARAMETERS_FILE_NAME}' at {model_hyperparameters_path}.")
 
@@ -219,6 +236,7 @@ def upload(
         private: bool | None = False,
         commit_message: str | None = None,
         commit_description: str | None = None,
+        model_path_is_experiment_path: bool = True,
         **kwargs,
     ) -> bool:
         """Create an empty repo on the HuggingFace Hub and upload trained model artifacts to that repo.
@@ -242,6 +260,8 @@ def upload(
                 `f"Upload {path_in_repo} with huggingface_hub"`
             commit_description (`str` *optional*):
                 The description of the generated commit
+            model_path_is_experiment_path (`bool`, *optional*):
+                Whether the 'model_path' argument points to the experiment folder
         """
         # Validate upload parameters are in the right format
         HuggingFaceHub._validate_upload_parameters(
@@ -251,6 +271,7 @@ def upload(
             private,
             commit_message,
             commit_description,
+            model_path_is_experiment_path,
         )
 
         # Create empty model repo using repo_id, but it is okay if it already exists.
@@ -266,8 +287,12 @@ def upload(
         commit_description_weights: str | None = (
             f"{commit_description} (weights)" if commit_description else commit_description
         )
+        if model_path_is_experiment_path:
+            folder_path = os.path.join(model_path, "model", "model_weights")
+        else:
+            folder_path = os.path.join(model_path, "model_weights")
         upload_path_weights: CommitInfo = self.api.upload_folder(
-            folder_path=os.path.join(model_path, "model", "model_weights"),
+            folder_path=folder_path,
             repo_id=repo_id,
             repo_type=repo_type,
             commit_message=commit_message_weights,
@@ -281,8 +306,12 @@ def upload(
             commit_description_config: str | None = (
                 f"{commit_description} (config)" if commit_description else commit_description
             )
+            if model_path_is_experiment_path:
+                path_or_fileobj = os.path.join(model_path, "model", MODEL_HYPERPARAMETERS_FILE_NAME)
+            else:
+                path_or_fileobj = os.path.join(model_path, MODEL_HYPERPARAMETERS_FILE_NAME)
             upload_path_config: CommitInfo = self.api.upload_file(
-                path_or_fileobj=os.path.join(model_path, "model", MODEL_HYPERPARAMETERS_FILE_NAME),
+                path_or_fileobj=path_or_fileobj,
                 path_in_repo="ludwig_config.json",
                 repo_id=repo_id,
                 repo_type=repo_type,
@@ -334,6 +363,7 @@ def _validate_upload_parameters(
         private: bool | None = False,
         commit_message: str | None = None,
         commit_description: str | None = None,
+        model_path_is_experiment_path: bool = True,
     ):
         """Validate parameters before uploading trained model artifacts.
 
@@ -354,6 +384,8 @@ def _validate_upload_parameters(
             commit_description (str, optional): A description of the commit when uploading to version control
                 systems. Not used in the base class, but subclasses may use it for specific repository
                 implementations. Defaults to None.
+            model_path_is_experiment_path (bool, optional): Whether the 'model_path' argument points to the
+                experiment folder.
 
         Raises:
             ValueError: If the repo_id is too long.
@@ -368,6 +400,7 @@ def _validate_upload_parameters(
             private,
             commit_message,
             commit_description,
+            model_path_is_experiment_path,
         )
 
     def upload(

tests/ludwig/utils/test_upload_utils.py

@@ -176,3 +176,134 @@ def test_upload_to_hf_hub__validate_upload_parameters(
             )
         except Exception as exc:
             assert False, f'"HuggingFaceHub._validate_upload_parameters()" raised an exception: "{exc}".'
+
+
+@pytest.mark.parametrize(
+    "file_names,error_raised",
+    [
+        pytest.param(
+            [
+                "pytorch_model.bin",
+            ],
+            None,
+            id="pretrained_model_weights_bin",
+        ),
+        pytest.param(
+            [
+                "adapter_model.bin",
+            ],
+            None,
+            id="adapter_model_weights_bin_unmerged",  # backward compatibility for peft versions < 0.7.0
+        ),
+        pytest.param(
+            [
+                "adapter_model.safetensors",
+            ],
+            None,
+            id="adapter_model_weights_safetensors_unmerged",
+        ),
+        pytest.param(
+            [
+                "adapter_model.bin",
+                "adapter_model.safetensors",
+            ],
+            None,
+            id="adapter_model_weights_bin_and_safetensors_unmerged",  # backward compatibility for peft versions < 0.7.0
+        ),
+        pytest.param(
+            [
+                "pytorch_model.bin",
+                "adapter_model.safetensors",
+            ],
+            None,
+            id="pretrained_model_weights_bin_and_adapter_model_weights_safetensors_merged",
+        ),
+        pytest.param(
+            [],
+            (
+                ValueError,
+                "Can't find model weights at {model_weights_path}. Trained model weights should either be saved as `pytorch_model.bin` for regular model training, or have `adapter_model.bin`or `adapter_model.safetensors` if using parameter efficient fine-tuning methods like LoRA.",  # noqa E501
+            ),
+            id="model_weights_missing",
+        ),
+        pytest.param(
+            [
+                "pytorch_model.safetensors",
+            ],
+            (
+                ValueError,
+                "Can't find model weights at {model_weights_path}. Trained model weights should either be saved as `pytorch_model.bin` for regular model training, or have `adapter_model.bin`or `adapter_model.safetensors` if using parameter efficient fine-tuning methods like LoRA.",  # noqa E501
+            ),
+            id="model_weights_unexpected_name_format_combination",
+        ),
+        pytest.param(
+            [
+                "pytorch_model.unkn",
+            ],
+            (
+                ValueError,
+                "Can't find model weights at {model_weights_path}. Trained model weights should either be saved as `pytorch_model.bin` for regular model training, or have `adapter_model.bin`or `adapter_model.safetensors` if using parameter efficient fine-tuning methods like LoRA.",  # noqa E501
+            ),
+            id="model_weights_unrecognized_format",
+        ),
+        pytest.param(
+            [
+                "unknown_model.safetensors",
+            ],
+            (
+                ValueError,
+                "Can't find model weights at {model_weights_path}. Trained model weights should either be saved as `pytorch_model.bin` for regular model training, or have `adapter_model.bin`or `adapter_model.safetensors` if using parameter efficient fine-tuning methods like LoRA.",  # noqa E501
+            ),
+            id="model_weights_unrecognized_name",
+        ),
+    ],
+)
+@pytest.mark.unit
+def test_upload_to_hf_hub__validate_upload_parameters2(
+    output_directory_manager, file_names: list[str], error_raised: tuple[type, str] | None
+):
+    """Test "HuggingFaceHub._validate_upload_parameters()", which is executed in the path of upload to HuggingFace
+    Hub; for example: `upload hf_hub -repo_id "hf-account/repo-name" --model_path.
+
+    /content/results/api_experiment_run`.
+
+    Each test case consists of: 1) Populating the temporary output directory ("training_results_output) with zero or
+    more test model weights file; 2) Executing "HuggingFaceHub._validate_upload_parameters()"; and 3) Asserting on
+    presence/absence of errors.
+
+    This test differs from the previous test (`test_upload_to_hf_hub__validate_upload_parameters`) by passing the
+    `model` folder to the `model_path` argument of `HuggingFaceHub._validate_upload_parameters()`. The previous test
+    passed the `experiment` folder to the `model_path` argument. Also note the additional argument
+    `model_path_is_experiment_path=False` passed to `HuggingFaceHub._validate_upload_parameters`.
+    """
+    output_directory: str = output_directory_manager
+    _build_fake_model_repo(
+        destination_directory=output_directory, experiment_name="my_simple_experiment_run", file_names=file_names
+    )
+
+    model_path: pathlib.Path = pathlib.Path(output_directory) / "my_simple_experiment_run" / "model"
+    model_weights_path: pathlib.Path = pathlib.Path(model_path / "model_weights")
+
+    repo_id: str = "test_account/test_repo"
+    model_path: str = str(model_path)
+    if error_raised:
+        error_class: type  # noqa [F842]  # incorrect flagging of "local variable is annotated but never used
+        error_message: str  # noqa [F842]  # incorrect flagging of "local variable is annotated but never used
+        error_class, error_message = error_raised
+        with pytest.raises(error_class) as excinfo:
+            HuggingFaceHub._validate_upload_parameters(
+                repo_id=repo_id,
+                model_path=model_path,
+                model_path_is_experiment_path=False,
+            )
+
+        assert str(excinfo.value) == error_message.format(model_weights_path=model_weights_path)
+    else:
+        try:
+            HuggingFaceHub._validate_upload_parameters(
+                repo_id=repo_id,
+                model_path=model_path,
+                model_path_is_experiment_path=False,
+            )
+        except Exception as exc:
+            assert False, f'"HuggingFaceHub._validate_upload_parameters()" raised an exception: "{exc}".'