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

[burn-import] ArgType Shape dosen't seem to match specs #919

Closed
QuantumEntangledAndy opened this issue Nov 1, 2023 · 3 comments
Closed
Labels

Comments

@QuantumEntangledAndy
Copy link

Describe the bug

Shape: According to the onnx reference is defined as a vector of integers

In burn_import ArgType::Shape is defined as ArgType::Shape(usize) as in it only stores the number of dimensions not the shape.

Expected behavior

Define ArgType::Shape(usize) as ArgType::Shape(Vec<usize>)

This is needed for gather op I am working on which seems to be able to accept both tensors and shapes of a tensor

Notes

If I get the time I'll work up a PR but thought I'd post here first for comments about what it is like this

@QuantumEntangledAndy QuantumEntangledAndy changed the title [burn-import] [burn-import] ArgType Shape dosen't seem to match specs Nov 1, 2023
@QuantumEntangledAndy
Copy link
Author

Looking at this close I think it would be better to make the output of the Shape convert as

ArgType::Tensor(TensorType {
            elem_type: ElementType::Int64,
            dim: 1,
            shape: tensor.shape.map(|shapes| vec![shapes.len()]),
        });

No need for a seperate ArgType as the output of Shape is a tensor of integers, which is already well described

@louisfd louisfd added the onnx label Nov 7, 2023
@antimora
Copy link
Collaborator

@CohenAriel has implemented ONNX Gather, see #947.

@QuantumEntangledAndy let us know if this issue should still be open.

@antimora
Copy link
Collaborator

Closing it for now. Please re-open it if still needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants