-
Notifications
You must be signed in to change notification settings - Fork 1.9k
MLContext.Model.Load overloads that take file path instead of stream. #3008
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
Changes from all commits
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 |
---|---|---|
|
@@ -182,12 +182,25 @@ public ITransformer Load(Stream stream, out DataViewSchema inputSchema) | |
} | ||
|
||
/// <summary> | ||
/// Load the model and its input schema from the stream. | ||
/// Load the model and its input schema from the file. | ||
/// </summary> | ||
/// <param name="modelPath">Path to model.</param> | ||
/// <param name="inputSchema">Will contain the input schema for the model. If the model was saved using older APIs | ||
/// it may not contain an input schema, in this case <paramref name="inputSchema"/> will be null.</param> | ||
/// <returns>The loaded model.</returns> | ||
public ITransformer Load(string modelPath, out DataViewSchema inputSchema) | ||
{ | ||
using (var stream = File.OpenRead(modelPath)) | ||
return Load(stream, out inputSchema); | ||
} | ||
|
||
/// <summary> | ||
/// Load the model from the stream. | ||
/// </summary> | ||
/// <param name="stream">A readable, seekable stream to load from.</param> | ||
/// <returns>A model of type <see cref="CompositeDataLoader{IMultiStreamSource, ITransformer}"/> containing the loader | ||
/// and the transformer chain.</returns> | ||
public IDataLoader<IMultiStreamSource> Load(Stream stream) | ||
public IDataLoader<IMultiStreamSource> LoadDataLoader(Stream stream) | ||
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. Should we make a similar change for the /// <summary>
/// Save the model to the file.
/// </summary>
/// <param name="model">The trained model to be saved.</param>
/// <param name="filePath">Path where model should be saved.</param>
public void Save<TSource>(IDataLoader<TSource> model, string filePath)
{
using (var stream = File.Create(filePath))
Save(model, stream);
} These are no longer symmetric. 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. Yes but this is Load API PR. I have created another PR to rename this for Save API #3019 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. @eerhardt @TomFinley feels we shouldn't change for Save because in .Net we have ReadChar, ReadInt, ReadDouble, etc but Write() is just one loaded set of methods. Thoughts? 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. I dont quite follow is there more discussion on github? In reply to: 267100385 [](ancestors = 267100385) |
||
{ | ||
_env.CheckValue(stream, nameof(stream)); | ||
|
||
|
@@ -205,6 +218,18 @@ public IDataLoader<IMultiStreamSource> Load(Stream stream) | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Load the model from the file. | ||
/// </summary> | ||
/// <param name="modelPath">Path to model.</param> | ||
/// <returns>A model of type <see cref="CompositeDataLoader{IMultiStreamSource, ITransformer}"/> containing the loader | ||
/// and the transformer chain.</returns> | ||
public IDataLoader<IMultiStreamSource> LoadDataLoader(string modelPath) | ||
{ | ||
using (var stream = File.OpenRead(modelPath)) | ||
return LoadDataLoader(stream); | ||
} | ||
|
||
/// <summary> | ||
/// Load a transformer model and a data loader model from the stream. | ||
/// </summary> | ||
|
@@ -215,7 +240,7 @@ public ITransformer LoadWithDataLoader(Stream stream, out IDataLoader<IMultiStre | |
{ | ||
_env.CheckValue(stream, nameof(stream)); | ||
|
||
loader = Load(stream); | ||
loader = LoadDataLoader(stream); | ||
if (loader is CompositeDataLoader<IMultiStreamSource, ITransformer> composite) | ||
{ | ||
loader = composite.Loader; | ||
|
@@ -224,6 +249,18 @@ public ITransformer LoadWithDataLoader(Stream stream, out IDataLoader<IMultiStre | |
return new TransformerChain<ITransformer>(); | ||
} | ||
|
||
/// <summary> | ||
/// Load a transformer model and a data loader model from the file. | ||
/// </summary> | ||
/// <param name="modelPath">Path to model.</param> | ||
/// <param name="loader">The data loader from the model stream.</param> | ||
/// <returns>The transformer model from the model stream.</returns> | ||
public ITransformer LoadWithDataLoader(string modelPath, out IDataLoader<IMultiStreamSource> loader) | ||
{ | ||
using (var stream = File.OpenRead(modelPath)) | ||
return LoadWithDataLoader(stream, out loader); | ||
} | ||
|
||
/// <summary> | ||
/// Create a prediction engine for one-time prediction. | ||
/// </summary> | ||
|
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.
Is it worth calling out the behavior for older models having a null input schema? That should never be the case now right?