Skip to content

TensorFlow transform would throw on non-vector input #1542

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

Closed
TomFinley opened this issue Nov 6, 2018 · 1 comment · Fixed by #3768
Closed

TensorFlow transform would throw on non-vector input #1542

TomFinley opened this issue Nov 6, 2018 · 1 comment · Fixed by #3768
Assignees
Labels
bug Something isn't working need info This issue needs more info before triage P1 Priority of the issue for triage purpose: Needs to be fixed soon.

Comments

@TomFinley
Copy link
Contributor

In the process of working on #1533, I found the following code in the tensorflow transform.

_isInputVector[i] = type.IsVector;
var expectedType = TensorFlowUtils.Tf2MlNetType(_parent.TFInputTypes[i]);
if (type.ItemType != expectedType)
throw _host.ExceptSchemaMismatch(nameof(inputSchema), "input", _parent.Inputs[i], expectedType.ToString(), type.ToString());
var originalShape = _parent.TFInputShapes[i];
var shape = originalShape.ToIntArray();
var colTypeDims = Enumerable.Range(0, type.AsVector.DimCount + 1).Select(d => d == 0 ? 1 : (long)type.AsVector.GetDim(d - 1)).ToArray();
if (shape == null)
_fullySpecifiedShapes[i] = new TFShape(colTypeDims);
else if (type.AsVector.DimCount == 1)

Consider the above code. We have at line 834 this assignment to a boolean value depending on whether the input is of type vector, or not. This strongly suggests that the transform can accomodate non-vector types. However at lines 841 and 844, we have this type.AsVector.DimCount. Now, AsVector will be null in the case where the type is not a vector of course, so this would throw a null reference exception if we were to ever feed this transform a non-vector value.

So, there's something wrong here. Unfortunately the intent of what the author meant to write is somewhat hidden from me, so perhaps whoever wrote this code could check this out. Maybe even write a test to test this condition.

@TomFinley TomFinley added the bug Something isn't working label Nov 6, 2018
@najeeb-kazmi
Copy link
Member

@abgoswam @yaeldekel Can you provide details on the original intent of this code?

@najeeb-kazmi najeeb-kazmi added the need info This issue needs more info before triage label Nov 7, 2018
@wschin wschin added the P0 Priority of the issue for triage purpose: IMPORTANT, needs to be fixed right away. label May 21, 2019
@codemzs codemzs added P1 Priority of the issue for triage purpose: Needs to be fixed soon. and removed P0 Priority of the issue for triage purpose: IMPORTANT, needs to be fixed right away. labels May 22, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working need info This issue needs more info before triage P1 Priority of the issue for triage purpose: Needs to be fixed soon.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants