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

Implement the ConvTranspose operation #182

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

Conversation

mayjs
Copy link
Contributor

@mayjs mayjs commented Aug 2, 2023

I have a first draft of a working ConvTranspose implementation.
Still want to add support for padding and maybe groups before we merge this.
Also should probably handle some more edgecases for unsupported attribute values to at least throw an informative error.

@mayjs mayjs force-pushed the feature/implement_conv_transpose branch 4 times, most recently from 63ce8e5 to 8ec1e4e Compare August 6, 2023 14:44
@mayjs mayjs force-pushed the feature/implement_conv_transpose branch 2 times, most recently from 8a981e1 to 4b3b6e8 Compare August 12, 2023 19:15
@mayjs mayjs changed the title WIP: Implement the ConvTranspose operation Implement the ConvTranspose operation Aug 12, 2023
@mayjs mayjs force-pushed the feature/implement_conv_transpose branch from 4b3b6e8 to cc04616 Compare August 12, 2023 19:30
@mayjs
Copy link
Contributor Author

mayjs commented Aug 12, 2023

Okay, I think we have the most important features of ConvTranspose supported now.

We are missing support for 1D and 3D, output_padding, auto_pad, dilations, and group.
I'm also not sure if this would work with anything other than f32, so I put in a check for the datatype as well.

@pixelspark I didn't run the ONNX backend tests locally, so we'll have to see if the CI check passes.

Do you want me to annotate the limitations in the support table in the README?

@mayjs
Copy link
Contributor Author

mayjs commented Aug 12, 2023

Also, as I mentioned before, this was the missing step for me to run UNet. I have successfully used this to run a pretrained nind-denoise model.
We could at a test for UNet to repository as you already suggested in !183, but I'm not sure how we would handle the rather large ONNX file.

Creating a randomized graph might be a nice way to test the functionality but won't provide any further use to anyone.

@pixelspark
Copy link
Collaborator

@mayjs backend test appears to have passed?

Re the UNet sample: how large would the download size be for running the example/test? I might be able to host the files (I have a VPS with some traffic budget).

@mayjs
Copy link
Contributor Author

mayjs commented Aug 13, 2023

@pixelspark Hm, looking through the log files (https://github.com/webonnx/wonnx/actions/runs/5843079870/job/15844866120?pr=182), ConvTranspose is not listed in the loaded / supported operations. Not sure where the operations are registered, but it's looking like the ConvTranspose backend tests are not running. I'll look into that.

The model I'm using is about 57MB.
I plan to ship it as a part of NeuraTable in the future, so I'll need to figure out a solution for deployment anyway.
My current plan is to create a fork of nind-denoise and upload the pretrained ONNX file as a release artifact, so maybe a test runner for wonnx could also just pull it from there.
Another issue is the license; I'm not sure what the implications would be if a wonnx test loads a GPL licensed model during testing - strictly speaking, the model is not integrated into the software, but I'm really unsure about the legal situation here.

@mayjs
Copy link
Contributor Author

mayjs commented Aug 13, 2023

Hm, maybe I'm misunderstanding something.
I thought that adding the test cases to test_onnx_backend.py would let them run in the CI.
I took the name of the tests from here: https://github.com/onnx/onnx/blob/rel-1.13.1/onnx/backend/test/case/node/convtranspose.py

Any idea why the test is not executed? Or am I just not reading the output correctly?

@pixelspark
Copy link
Collaborator

Probably the easiest way to fix this would be to run the tests locally... I can try later if necessary.

@mayjs
Copy link
Contributor Author

mayjs commented Aug 15, 2023

Probably the easiest way to fix this would be to run the tests locally... I can try later if necessary.

Yeah I guess that would be easier - I didn't have time to setup the required Python environment and I suspect that it might take a bit of work for me since I'm on NixOS, so if you have time to take a look at this that would be great :)

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