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

feat: Add support for slice operator #171

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

raphaelmenges
Copy link

@raphaelmenges raphaelmenges commented Jun 2, 2023

Hello 👋!

I need the slice operator (v11) to infer one of our models. I have seen in issue #153 that the slice operator is not yet implemented in wonnx, thus I plan to tackle the task. I have low experience in Rust, yet high in C++, and medium experience in compute shaders, mostly OpenGL 4. Therefore, I would suggest to treat this PR as basis to support me in understanding how to approach this venture. I have read the instructions from your README and attempted to get into the code of the project.

I already fail at extracting the attribute values from a slice node of our model. It looks like all attributes according to the ONNX specification [1] are integer vectors. I have spotted in

let dilations = node.get_attribute_value("dilations", Some(vec![1, 1]))?;
how you extract such an attribute. However, when I apply the same scheme in lines 463-466 of compiler.rs it just returns the default values I defined [2]. What am I missing?

I hope you are fine with my approach to this pull request. Otherwise I can also provide specific issues if desired.

Cheers,
Raphael

[1] https://onnx.ai/onnx/operators/onnx__Slice.html#slice-11
[2] https://github.com/semanux/wonnx/blob/05202639b25d4020478135dbdc717a04ea08a16d/wonnx/src/compiler.rs#L463

@pixelspark
Copy link
Collaborator

Hi @raphaelmenges, happy to help!

The issue with not returning the correct attribute may be related to the ONNX file you are using to test. Therefore I would suggest to add an as-simple-as-possible test case somewhere (have a look at the other test functions to see how to build an ONNX model from code). Then, either it works (and your ONNX file has an issue? Perhaps you can share it, or a simplified version, here) or it doesn't (in which case I can run it too and see what's wrong). It may also have something to do with the supported data types (vector attributes can be stored in different ways and not all of them may be implemented yet).

@raphaelmenges
Copy link
Author

I have added a simple test for slice that should reproduce the first example of the ONNX specification of slice. I am still a bit unsure about the data layout wonnx expects and just deduced it from other parts of it. Is there any reference or other project with similar data layout to better understand how shapes, chunks etc. relate?

In the test, the attributes are correctly read in compiler.rs. Thus, our model might be the problem. I checked the slice operator instances in Netron and found for example const_starts__7 as value for starts. I will talk to my team who has created the model to get a better understanding, yet any hints from your side are welcome as well.

If the slice operator remains the last barrier to interfere our model I am eager to further investigate its implementation!

@pixelspark
Copy link
Collaborator

I have added a simple test for slice that should reproduce the first example of the ONNX specification of slice. I am still a bit unsure about the data layout wonnx expects and just deduced it from other parts of it. Is there any reference or other project with similar data layout to better understand how shapes, chunks etc. relate?

Not sure what your question exactly is, but a short summary:

  • Shape refers to the number and size of dimensions (and hence number of elements) in a tensor (either input, output or attribute of a node). ONNX models contains shape information for all fixed tensors (i.e. weights) but does not require all inputs, outputs or intermediate values to have a shape specified (instead they can be inferred when input shapes are set at runtime). WONNX contains shape inference code but it is recommended that you use ONNX models with full shape information unless you need the shape inference to happen at runtime (due to e.g. dynamic input sizes).
  • Chunks are derived from shapes and indicate the number of elements to 'skip' in later dimensions. I.e., if a shape is [3,4,5] then chunks might be [3, 3*4, 3*4*5].
  • An operator is implemented as a shader program, of which many instances are run in parallel at the same time.
  • Each instance gets a 'thread index' which is a tuple of (x,y,z) 'coordinates' indicating which part of the data that instance works on (note that the ranges of x,y,z are limited!)
  • Depending on the operation a single shader instance runs on a specific span of the input tensor(s), basically x, y and/or z multiplied by the chunks indices calculated earlier.

In the test, the attributes are correctly read in compiler.rs. Thus, our model might be the problem. I checked the slice operator instances in Netron and found for example const_starts__7 as value for starts. I will talk to my team who has created the model to get a better understanding, yet any hints from your side are welcome as well.

According to the documentation, the starts information is contained in an input rather than an attribute. So instead of using get_attribute, you will receive the data as the second input to the operator (in fact Slice can have between 3...5 inputs).

The WONNX optimizer in some cases moves such inputs to attributes for some operators if they are constant. Of course the input may also be calculated dynamically (and for some ops that is not supported).

If the slice operator remains the last barrier to interfere our model I am eager to further investigate its implementation!

@raphaelmenges
Copy link
Author

raphaelmenges commented Jun 5, 2023

Thank you @pixelspark for the extensive explanation! Your summary provides me indeed a good insight into the data structures which I would have struggled to gain from the code base alone. I would suggest to add these explanations to the contributor's section of the README of wonnx.

the starts information is contained in an input rather than an attribute.

I can confirm your finding. I have pushed according changes to my code. Our model seems to correctly report starts and ends parameters, great! I believe I have now all pieces of information at hand how to proceed 👌

PS: I have a question about the operator version. To me it seems that there is quite some leap between v11 and v13 of the Slice operator definition. For our model, it looks like we only need v11. Would it be fine to just implement v11 and throw an error in compiler.rs if v13 is requested? I believe the test case automatically assumes v13 however.

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