Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only generate model uuid when logging model #5167

Merged
merged 6 commits into from Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions mlflow/models/model.py
Expand Up @@ -54,7 +54,7 @@ def __init__(
self.flavors = flavors if flavors is not None else {}
self.signature = signature
self.saved_input_example_info = saved_input_example_info
self.model_uuid = uuid.uuid4().hex if model_uuid is None else model_uuid
self.model_uuid = model_uuid
self.__dict__.update(kwargs)

def __eq__(self, other):
Expand Down Expand Up @@ -186,7 +186,8 @@ def log(
with TempDir() as tmp:
local_path = tmp.path("model")
run_id = mlflow.tracking.fluent._get_or_start_run().info.run_id
mlflow_model = cls(artifact_path=artifact_path, run_id=run_id)
model_uuid = uuid.uuid4().hex
mlflow_model = cls(artifact_path=artifact_path, run_id=run_id, model_uuid=model_uuid)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not set uuid here. Otherwise there're many other place we need also set uuid.
e.g.

mlflow_model = Model()

mlflow_model = Model()

etc.

I move the set uuid code into Model constructor again.

flavor.save_model(path=local_path, mlflow_model=mlflow_model, **kwargs)
mlflow.tracking.fluent.log_artifacts(local_path, artifact_path)
try:
Expand Down
34 changes: 14 additions & 20 deletions tests/pyfunc/test_model_export_with_loader_module_and_data_path.py
Expand Up @@ -85,6 +85,16 @@ def model_path(tmpdir):
return os.path.join(str(tmpdir), "model")


def _is_valid_uuid(val):
import uuid

try:
uuid.UUID(str(val))
return True
except ValueError:
return False


@pytest.mark.large
def test_model_save_load(sklearn_knn_model, iris_data, tmpdir, model_path):
sk_model_path = os.path.join(str(tmpdir), "knn.pkl")
Expand All @@ -102,13 +112,17 @@ def test_model_save_load(sklearn_knn_model, iris_data, tmpdir, model_path):

reloaded_model_config = Model.load(os.path.join(model_path, "MLmodel"))
assert model_config.__dict__ == reloaded_model_config.__dict__
assert model_config.model_uuid is not None and _is_valid_uuid(model_config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert model_config.model_uuid is not None and _is_valid_uuid(model_config)
assert model_config.model_uuid is not None
assert _is_valid_uuid(model_config.model_uuid)

assert mlflow.pyfunc.FLAVOR_NAME in reloaded_model_config.flavors
assert mlflow.pyfunc.PY_VERSION in reloaded_model_config.flavors[mlflow.pyfunc.FLAVOR_NAME]
reloaded_model = mlflow.pyfunc.load_pyfunc(model_path)
np.testing.assert_array_equal(
sklearn_knn_model.predict(iris_data[0]), reloaded_model.predict(iris_data[0])
)

reloaded_model_config2 = Model.load(os.path.join(model_path, "MLmodel"))
assert reloaded_model_config.model_uuid == reloaded_model_config2.model_uuid


@pytest.mark.large
def test_signature_and_examples_are_saved_correctly(sklearn_knn_model, iris_data):
Expand Down Expand Up @@ -556,26 +570,6 @@ def test_column_schema_enforcement_no_col_names():
assert pyfunc_model.predict(d).equals(pd.DataFrame(d))


def _is_valid_uuid(val):
import uuid

try:
uuid.UUID(str(val))
return True
except ValueError:
return False


def test_model_uuid():
m = Model()
assert m.model_uuid is not None
assert _is_valid_uuid(m.model_uuid)
m_dict = m.to_dict()
assert m_dict["model_uuid"] == m.model_uuid
m2 = Model.from_dict(m_dict)
assert m2.model_uuid == m.model_uuid


def test_tensor_schema_enforcement_no_col_names():
m = Model()
input_schema = Schema([TensorSpec(np.dtype(np.float32), (-1, 3))])
Expand Down