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

Change types for some tests to correct types #315

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rsuderman
Copy link
Contributor

No description provided.

@rsuderman rsuderman requested a review from ScottTodd August 2, 2024 19:31
--input=3x4xui16=@input_0.bin
--input=3x4xbf16=@input_0.bin
Copy link
Member

Choose a reason for hiding this comment

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

These files are generated. The type conversion from onnx happens here:

# map numpy dtype -> (iree dtype, struct.pack format str)
dtype_map = {
np.dtype("int64"): ("si64", "q"),
np.dtype("uint64"): ("ui64", "Q"),
np.dtype("int32"): ("si32", "i"),
np.dtype("uint32"): ("ui32", "I"),
np.dtype("int16"): ("si16", "h"),
np.dtype("uint16"): ("ui16", "H"),
np.dtype("int8"): ("si8", "b"),
np.dtype("uint8"): ("ui8", "B"),
np.dtype("float64"): ("f64", "d"),
np.dtype("float32"): ("f32", "f"),
np.dtype("float16"): ("f16", "e"),
np.dtype("bool"): ("i1", "?"),
}
t = convert_io_proto(test_input, model.graph.input[i].type)
if t is None:
return False
input_path = (imported_dir_path / test_input.stem).with_suffix(".npy")
np.save(input_path, t) # Only for ref, actual comparison with .bin
ss = get_shape_string(t)

is that code not handling a case here? I'd expect the WARNING: unsupported data type in get_shape_string to be hit 🤔

Copy link
Member

Choose a reason for hiding this comment

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

While we can run the generator once and then manually update tests like this, I don't want for future runs of the generator to need to manually make these same changes, so I'd prefer for the generator to be taught to make these changes itself.

If we're losing information across the onnx -> numpy -> iree translation, we could add some special cases in the generator script (similar to how I suggested adding per-test tolerances on #311 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would need to dig into the proto but my guess is that they just identify themselves at uint16 with an internal bitcast. bf16 is a bit weird because from a data standpoint its just a truncated f32 and is basically treated that way. This results in buffers often just being passed around at uint16

@ScottTodd
Copy link
Member

Oh, can you sync past 53bedfb ? The important detail there is the skip_compile_tests additions (to avoid timeouts).

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.

2 participants