Tensorflow GetModelSchema should support unfrozen models #2112
Conversation
| var mlContext = new MLContext(seed: 1, conc: 1); | ||
| var loadModelSchema = TensorFlowUtils.GetModelSchema(mlContext, modelLocation); | ||
| Assert.Equal(335, loadModelSchema.Count); |
There was a problem hiding this comment.
Assert.Equal(335, loadModelSchema.Count [](start = 12, length = 39)
am curious , what does this count signify ? #Closed
There was a problem hiding this comment.
It's all the nodes inside tensor flow graph.
I just wanted to test GetModelSchema on unfrozen model.
In reply to: 246930752 [](ancestors = 246930752)
| /// <param name="modelFile">The name of the file containing the TensorFlow model. Currently only frozen model | ||
| /// format is supported.</param> | ||
| public static Schema GetModelSchema(IExceptionContext ectx, string modelFile) | ||
| public static Schema GetModelSchema(IHostEnvironment env, string modelFile) |
There was a problem hiding this comment.
modelFile [](start = 73, length = 9)
for consistency, can we call this modelPath #Closed
| } | ||
|
|
||
| /// <summary> | ||
| /// This is a convenience method for iterating over the nodes of a TensorFlow model graph. It | ||
| /// iterates over the columns of the <see cref="ISchema"/> returned by <see cref="GetModelSchema(IExceptionContext, string)"/>, | ||
| /// iterates over the columns of the <see cref="ISchema"/> returned by <see cref="GetModelSchema(IHostEnvironment, string)"/>, | ||
| /// and for each one it returns a tuple containing the name, operation type, column type and an array of input node names. | ||
| /// This method is convenient for filtering nodes based on certain criteria, for example, by the operation type. | ||
| /// </summary> | ||
| /// <param name="modelFile"></param> | ||
| /// <returns></returns> | ||
| public static IEnumerable<(string, string, ColumnType, string[])> GetModelNodes(string modelFile) |
There was a problem hiding this comment.
modelFile [](start = 95, length = 9)
for consistency, can we call this modelPath #Closed
| } | ||
|
|
||
| /// <summary> | ||
| /// This is a convenience method for iterating over the nodes of a TensorFlow model graph. It | ||
| /// iterates over the columns of the <see cref="ISchema"/> returned by <see cref="GetModelSchema(IExceptionContext, string)"/>, | ||
| /// iterates over the columns of the <see cref="ISchema"/> returned by <see cref="GetModelSchema(IHostEnvironment, string)"/>, | ||
| /// and for each one it returns a tuple containing the name, operation type, column type and an array of input node names. | ||
| /// This method is convenient for filtering nodes based on certain criteria, for example, by the operation type. | ||
| /// </summary> | ||
| /// <param name="modelFile"></param> |
There was a problem hiding this comment.
/// [](start = 8, length = 36)
some description ? #Closed
| @@ -75,27 +75,26 @@ internal static Schema GetModelSchema(IExceptionContext ectx, TFGraph graph, str | |||
| /// of kind <see cref="OpType"/>, indicating the operation type of the node, and if that node has inputs in the graph, | |||
| /// it contains metadata of kind <see cref="InputOps"/>, indicating the names of the input nodes. | |||
| /// </summary> | |||
| /// <param name="ectx">An <see cref="IExceptionContext"/>.</param> | |||
| /// <param name="env">The environment to use.</param> | |||
| /// <param name="modelFile">The name of the file containing the TensorFlow model. Currently only frozen model | |||
There was a problem hiding this comment.
Currently only frozen model [](start = 90, length = 27)
so this would need to be updated i presume #Closed
| { | ||
| var schema = GetModelSchema(null, modelFile); | ||
| var schema = GetModelSchema(new MLContext(), modelPath); |
There was a problem hiding this comment.
This is not correct. If you need an IHostEnvironment, you should be taking one as a parameter. It is wrong to create new environment (MLContext) out of thin air.
/cc @TomFinley
There was a problem hiding this comment.
This is only used as an IExceptionContext, so if we remove the check that env!=null on line 325, we can pass null.
In reply to: 247276711 [](ancestors = 247276711)
|
@Ivanidzo4ka . I am seeing sporadic failures in the TF tests . On the local machine I get a pop-up saying .NET Core Host stopped working. If I skip the TF tests, the build goes through fine. Could this be related to this change ? Note: the failures are sporadic. |
| public static Schema GetModelSchema(IExceptionContext ectx, string modelFile) | ||
| /// <param name="env">The environment to use.</param> | ||
| /// <param name="modelPath">Model to load.</param> | ||
| public static Schema GetModelSchema(IHostEnvironment env, string modelPath) |
There was a problem hiding this comment.
env [](start = 61, length = 3)
This is only used as an IExceptionContext, so instead of changing it here to IHostEnvironment, you can change LoadTensorFlowModel and GetSession to take an IExceptionContext.
ghost
locked as resolved and limited conversation to collaborators
Fix #2102