Fixed OnnxTransformer output column mapping. #5192
Conversation
…et and ONNX itself.
| [OnnxFact] | ||
| public void OnnxModelOutputDifferentOrder() | ||
| { |
There was a problem hiding this comment.
Maybe you can refactor both tests into one, as they're very similar. And include an [InlineData(true) with OnnxModelOutputDifferentOrder(bool checkSubset)?
There was a problem hiding this comment.
I'll combine them. I dont think the [InlineData] is needed since I'm testing the output ordering and not the checkSubset` itself though.
There was a problem hiding this comment.
I misunderstood what you were saying first. I get it now. The issue with that is that with the subset I dont compare all the columns and with the full column set I do. I just combined the tests and moved the subset pipeline and checking below the full one.
Codecov Report
@@ Coverage Diff @@
## master #5192 +/- ##
========================================
Coverage 73.32% 73.32%
========================================
Files 1007 1007
Lines 187911 188015 +104
Branches 20241 20243 +2
========================================
+ Hits 137790 137871 +81
- Misses 44595 44622 +27
+ Partials 5526 5522 -4
|
| /// <summary> | ||
| /// In the case that the ML.Net user wants a subset of columns or lists the columns in a different order then specified in the ONNX model, | ||
| /// we need to map from the ONNX model index to the ML.Net column index. This method does that mapping for us. | ||
| /// </summary> |
There was a problem hiding this comment.
nit: This is mapping the dataview column index to onnx output index.
ghost
locked as resolved and limited conversation to collaborators
Our OnnxTransformer has overrides that let you specify a subset of output columns, all the output columns, or let it figure it out from the onnx model. The issue was that if you manually specified either a subset of the columns, or all the columns, and did it in an order different then the onnx model, our transformer would not do the mapping correctly and it would try and access the wrong column. This usually resulted in an error that the types didn't match, but when types matched it just returned wrong data.
This PR fixes that issue by doing the correct mapping regardless of the order they are provided in.