Skip to content
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

[Web] Model returning Float32Array result fails to build a Tensor #16622

Closed
adam-harwood opened this issue Jul 7, 2023 · 5 comments
Closed
Labels
api:Javascript issues related to the Javascript API platform:web issues related to ONNX Runtime web; typically submitted using template

Comments

@adam-harwood
Copy link

Describe the issue

I am running inference on an ONNX model using onnxruntime-node. The model runs fine against onnxruntime-web. When run on Node (as Javascript, not Typescript), I receive the following exception from session.run():

A float32 tensor's data must be type of function Float32Array() { [native code] }

The error is being thrown from this line.

I have stepped through with the debugger, and found that the data looks like this:
image

I believe this object should pass the check on this line. It doesn't though, and falls through to throwing an error.

I believe the issue here is the way that type checking is done on Float32Arrays in Javascript. I don't think you can compare the prototype like that, since they're constructed from functions, so instead should be done against the constructor attribute. I suspect it currently only works in Typescript?

If you can confirm this is right I'm happy to open a PR to check both prototype and constructor, but would like some guidance first. The change to make I think would be to change this line from:

} else if (arg1 instanceof typedArrayConstructor) {

to

} else if (arg1 instanceof typedArrayConstructor || arg1.constructor.name === typedArrayConstructor.name) {

To reproduce

I can't provide the model, but I suspect using onnxruntime-node with any model that produces a Float32Array result will cause the issue.

Urgency

This issue blocks us (Guru) from launching a significant feature.

ONNX Runtime Installation

Released Package

ONNX Runtime Version or Commit ID

1.15.1

Execution Provider

Other / Unknown

@adam-harwood adam-harwood added the platform:web issues related to ONNX Runtime web; typically submitted using template label Jul 7, 2023
@github-actions github-actions bot added the api:Javascript issues related to the Javascript API label Jul 7, 2023
@fs-eire
Copy link
Contributor

fs-eire commented Jul 7, 2023

I cannot reproduce this issue. Could you share your JS code ( not model ) so that I can try?

@fs-eire
Copy link
Contributor

fs-eire commented Jul 7, 2023

the code if (arg1 instanceof typedArrayConstructor) in the context is same to if (arg1 instanceof Float32Array). If arg1 is created by either new Float32Array(...) or Float32Array.from(...), this condition should always be true. I need to know how this error happens - my guess is that there is a library that polyfills Float32Array so that the code takes different Float32Arrays between creating instance and performing this check.

@adam-harwood
Copy link
Author

Wow, so in trying to create the smallest possible repro to send to you, I ended up finding it was surprisingly Jest that is the culprit. I'm not the first person to hit the issue, and specifically around the onnxruntime.

The workaround listed there didn't work for me, so I ended up mocking Array.isArray in my Jest test to make it work. Posting here in case any future travellers hit this:

    const originalImplementation = Array.isArray;
    Array.isArray = jest.fn((type) => {
      if (type.constructor.name === "Float32Array" || type.constructor.name === "BigInt64Array") {
        return true;
      }

      return originalImplementation(type);
    });

Thanks for the help @fs-eire!

@sqs
Copy link

sqs commented Jan 7, 2024

@adam-harwood: Thank you! Your workaround also fixes the same issue in Vitest for me when using the vmThreads pool.

@Marviel
Copy link

Marviel commented Jan 11, 2024

Wow, so in trying to create the smallest possible repro to send to you, I ended up finding it was surprisingly Jest that is the culprit. I'm not the first person to hit the issue, and specifically around the onnxruntime.

The workaround listed there didn't work for me, so I ended up mocking Array.isArray in my Jest test to make it work. Posting here in case any future travellers hit this:

    const originalImplementation = Array.isArray;
    Array.isArray = jest.fn((type) => {
      if (type.constructor.name === "Float32Array" || type.constructor.name === "BigInt64Array") {
        return true;
      }

      return originalImplementation(type);
    });

Thanks for the help @fs-eire!

// @ts-ignore
Array.isArray = jest.fn((type) => {
  if (type.constructor && (type.constructor.name === "Float32Array" || type.constructor.name === "BigInt64Array")) {
    return true;
  }

  return originalImplementation(type);
});

Had to make a slight modification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api:Javascript issues related to the Javascript API platform:web issues related to ONNX Runtime web; typically submitted using template
Projects
None yet
Development

No branches or pull requests

4 participants