Updating the ApplyOnnxModel transform to meet the API parameter ordering standards #3086
Conversation
| /// <param name="gpuDeviceId">Optional GPU device ID to run execution on, <see langword="null" /> to run on CPU.</param> | ||
| /// <param name="fallbackToCpu">If GPU error, raise exception or fallback to CPU.</param> | ||
| public static OnnxScoringEstimator ApplyOnnxModel(this TransformsCatalog catalog, | ||
| string modelFile, | ||
| string outputColumnName, | ||
| string inputColumnName, |
There was a problem hiding this comment.
inputColumnName [](start = 19, length = 15)
should this default to the outputColumnName like the other inputColumns?
There was a problem hiding this comment.
There was a problem hiding this comment.
I went back and forth on this. I don't think we need to have this as an "in-place" transform because we expect models to create new columns, similar to the TensorFlow transform.
Codecov Report
@@ Coverage Diff @@
## master #3086 +/- ##
==========================================
+ Coverage 72.52% 72.53% +<.01%
==========================================
Files 807 807
Lines 144537 144537
Branches 16194 16194
==========================================
+ Hits 104831 104839 +8
+ Misses 35296 35291 -5
+ Partials 4410 4407 -3
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #3086 +/- ##
==========================================
+ Coverage 72.52% 72.53% +<.01%
==========================================
Files 807 807
Lines 144537 144537
Branches 16194 16194
==========================================
+ Hits 104831 104839 +8
+ Misses 35296 35291 -5
+ Partials 4410 4407 -3
|
| @@ -34,7 +34,7 @@ public static void Example() | |||
| var mlContext = new MLContext(); | |||
| var data = GetTensorData(); | |||
| var idv = mlContext.Data.LoadFromEnumerable(data); | |||
| var pipeline = mlContext.Transforms.ApplyOnnxModel(modelPath, new[] { outputInfo.Key }, new[] { inputInfo.Key }); | |||
| var pipeline = mlContext.Transforms.ApplyOnnxModel(new[] { outputInfo.Key }, new[] { inputInfo.Key }, modelPath); | |||
There was a problem hiding this comment.
modelPath [](start = 114, length = 9)
I see at line 20 where session is loaded. Does the ApplyOnnxModel reload the model? If it does then there needs to be a way to reuse the session object created above. #WontFix
There was a problem hiding this comment.
That is a good point. Can you file an issue on this? We can do this as we start to fix outstanding issues in the ONNX implementation.
In reply to: 268890248 [](ancestors = 268890248)
ghost
locked as resolved and limited conversation to collaborators
This PR updates the
ApplyOnnxModeltransform to meet the API standards: Moving tooutputColumnName,inputColumnName,modelFile.Fixes #3082