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

handle raw types correctly in helper.make_tensor #4262

Merged
merged 3 commits into from Jun 10, 2022

Conversation

garymm
Copy link
Contributor

@garymm garymm commented Jun 10, 2022

Before this change, the newly added test_make_int8_tensor would fail
because TENSOR_TYPE_TO_STORAGE_TENSOR_TYPE[INT8] = FLOAT32. But raw
INT8 values are packed and are not the same size as FLOAT32 values.

Signed-off-by: Gary Miguel garymiguel@microsoft.com

Before this change, the newly added `test_make_int8_tensor` would fail
because `TENSOR_TYPE_TO_STORAGE_TENSOR_TYPE[INT8] = FLOAT32`. But raw
INT8 values are packed and are not the same size as FLOAT32 values.

Signed-off-by: Gary Miguel <garymiguel@microsoft.com>
@garymm garymm requested a review from a team as a code owner June 10, 2022 00:12
@garymm garymm requested a review from jcwchen June 10, 2022 00:13
@garymm garymm added this to the 1.12 milestone Jun 10, 2022
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the quick fix! I can confirm this is the right way. Please fix flake8 failure.

I misunderstood so my fix (#4234) was incorrect --

Instead of storage type, actually make_tensor with raw directly uses its original NumPy type (defined in TENSOR_TYPE_TO_NP_TYPE) as bytes. Only TensorProto.FLOAT16 and TensorProto.BFLOAT16 are exceptions. Although now TensorProto.FLOAT16 uses uint16 as raw_data, the itemsize for uint16 and float16 is the same (2) so TensorProto.FLOAT16 is not an issue.

Thus, just like what this PR does, we only need to handle the special case TensorProto.BFLOAT16, because float32's itemsize is 4 and uint16 itemsize is 2 (there is a truncation when processing original float32 NumPy array).

Another finding is for making float16 data with raw, actually there is no need to convert original float16 NumPy array as np_array.view(dtype=np.uint16).flatten().tobytes() for raw_data, because their tobytes() are identical (See here). I will send another PR to improve it. Let's merge this PR first for upcoming ONNX 1.12.

Signed-off-by: Gary Miguel <garymiguel@microsoft.com>
@garymm
Copy link
Contributor Author

garymm commented Jun 10, 2022

@jcwchen please merge

@jcwchen jcwchen merged commit 4e24b63 into onnx:main Jun 10, 2022
jcwchen pushed a commit to jcwchen/onnx that referenced this pull request Jun 10, 2022
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
@garymm garymm deleted the storage_type branch June 10, 2022 21:03
jcwchen added a commit that referenced this pull request Jun 10, 2022
* Use op name rather than hard-coding Hann in window op doc strings. (#4248)

And strip trailing spaces.

Signed-off-by: Gary Miguel <garymiguel@microsoft.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* Fix: Window functions generate incorrect shape for with symmetric attribute (#4256)

* fix symmetric function definition, and add shape inference test

Signed-off-by: Sheil Kumar <smk2007@gmail.com>

* whitespace...

Signed-off-by: Sheil Kumar <smk2007@gmail.com>

* whitespace...

Signed-off-by: Sheil Kumar <smk2007@gmail.com>

* flake8 whitespace error

Signed-off-by: Sheil Kumar <smk2007@gmail.com>

* fix stft test

Signed-off-by: Sheil Kumar <smk2007@gmail.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* support truncation & fix big-endian support (#4238)

Signed-off-by: Ian Bearman <ianb@microsoft.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* Fix layer normalization's reference outputs (#4263)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* handle raw types correctly in helper.make_tensor (#4262)

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* bump to 1.12.0rc4 for cherry-picks

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

* Fix sub-graph generation for LN (#4268)

Signed-off-by: Wei-Sheng Chin <wschin@outlook.com>
Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>

Co-authored-by: Gary Miguel <garymiguel@microsoft.com>
Co-authored-by: Sheil Kumar <smk2007@gmail.com>
Co-authored-by: Ian Bearman <ianb@microsoft.com>
Co-authored-by: Wei-Sheng Chin <wschin@outlook.com>
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants