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
Add model_uuid
to Java Model
#5165
Changes from 7 commits
a93307d
209eca4
c659a89
3451c24
05ac51b
6b246d4
98ebc1e
290a9d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -6,6 +6,7 @@ | |||
import java.io.IOException; | ||||
import java.util.Map; | ||||
import java.util.Optional; | ||||
import java.util.UUID; | ||||
import org.mlflow.Flavor; | ||||
import org.mlflow.utils.FileUtils; | ||||
import org.mlflow.utils.SerializationUtils; | ||||
|
@@ -41,6 +42,9 @@ public static class Signature { | |||
@JsonProperty("input_example") | ||||
private Map<String, Object> input_example; | ||||
|
||||
@JsonProperty("model_uuid") | ||||
private String modelUuid; | ||||
|
||||
private String rootPath; | ||||
|
||||
/** | ||||
|
@@ -61,6 +65,11 @@ public static Model fromRootPath(String modelRootPath) throws IOException { | |||
public static Model fromConfigPath(String configPath) throws IOException { | ||||
File configFile = new File(configPath); | ||||
Model model = SerializationUtils.parseYamlFromFile(configFile, Model.class); | ||||
// Set the model uuid if it's absent. | ||||
if (!model.getModelUuid().isPresent()) { | ||||
String uuid = UUID.randomUUID().toString().replace("-", ""); | ||||
model.setModelUuid(uuid); | ||||
} | ||||
// Set the root path to the directory containing the configuration file. | ||||
// This will be used to create an absolute path to the serialized model | ||||
model.setRootPath(configFile.getParentFile().getAbsolutePath()); | ||||
|
@@ -82,6 +91,11 @@ public Optional<String> getRunId() { | |||
return Optional.ofNullable(this.runId); | ||||
} | ||||
|
||||
/** @return The MLflow model's uuid */ | ||||
public Optional<String> getModelUuid() { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: <4 Acronym naming convention in Java typically preserves the upper case acronym, particularly if the std library preserves. public Optional<String> getModelUUID() { There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @BenWilson2 Can we use
|
||||
return Optional.ofNullable(this.modelUuid); | ||||
} | ||||
|
||||
/** @return The path to the root directory of the MLflow model */ | ||||
public Optional<String> getRootPath() { | ||||
return Optional.ofNullable(this.rootPath); | ||||
|
@@ -104,4 +118,8 @@ public <T extends Flavor> Optional<T> getFlavor(String flavorName, Class<T> flav | |||
private void setRootPath(String rootPath) { | ||||
this.rootPath = rootPath; | ||||
} | ||||
|
||||
private void setModelUuid(String modelUuid) { | ||||
this.modelUuid = modelUuid; | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
flavors: | ||
mleap: | ||
mleap_version: 0.8.1 | ||
model_data: mleap/model | ||
utc_time_created: "2018-08-10 18:34:28.720095" | ||
run_id: c228016dea284522882b657d91eeffa6 | ||
artifact_path: model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we set a new ID under this case ? I prefer to keep it empty because the ID should be generated while logging model (and after model logged, the ID should be immutable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we set new generated ID here, suppose we load a old version model twice, we will get 2 in-memory model with different model ID, this break the rule of model ID being immutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WeichenXu123 If so, I think we need to fix the python code.
output:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I create a fixing PR for python side:
#5167