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.

Sign up for GitHub

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

Jump to bottom

Only generate model uuid when logging model #5167

Merged
merged 6 commits into from Dec 16, 2021
Merged

Only generate model uuid when logging model #5167

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2dc085
init
WeichenXu123 Dec 15, 2021
00db8ac
add test
WeichenXu123 Dec 15, 2021
1d279b6
update
WeichenXu123 Dec 15, 2021
c45d16d
update
WeichenXu123 Dec 15, 2021
fcba89d
update
WeichenXu123 Dec 15, 2021
2d4a3cb
updates
WeichenXu123 Dec 16, 2021
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
View file
Open in desktop
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/mlflow/pyfunc/__init__.py

Line 1072 in f9b5745

mlflow_model = Model()

mlflow/mlflow/catboost.py

Line 122 in f9b5745

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
10 changes: 0 additions & 10 deletions tests/pyfunc/test_model_export_with_loader_module_and_data_path.py
View file
Open in desktop
Expand Up @@ -566,16 +566,6 @@ def _is_valid_uuid(val):
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