Implement 3D and transposed 3D convolutions. #1945

booti386 · 2024-06-29T23:40:09Z

Hi!
Please review this PR carefully, I started working both on neural networks and this crate only 4 days ago.
I hope there are no major mistakes.

Checklist

Confirmed that run-checks all script has been executed.
Made sure the book is up to date with changes in this PR.

Related Issues/PRs

Fixes #1278: Conv3d feature request

Changes

Implement 3D and 3D transposed convolutions for all the supported backends.

antimora · 2024-06-30T16:43:47Z

Super cool!

Also I am glad you have added ONNX for Conv3d and implemented new ModuleDisplay!

I can review the PR but @nathanielsimard, @laggui and @louisfd are more qualified.

nathanielsimard · 2024-07-01T14:41:43Z

Thanks a lot @booti386 for the PR. @laggui can you check if the ONNX part is valid, and @louisfd can also validate kernel implementations.

louisfd

Looking good on my end!

antimora

I checked the import parts and they look good.

codecov · 2024-07-01T20:48:37Z

Codecov Report

Attention: Patch coverage is 93.33885% with 322 lines in your changes missing coverage. Please review.

Project coverage is 85.40%. Comparing base (a7efc10) to head (06bc6f2).
Report is 11 commits behind head on main.

❗ Current head 06bc6f2 differs from pull request most recent head 2c67448

Please upload reports for the commit 2c67448 to get more accurate results.

Files	Patch %	Lines
crates/burn-jit/src/kernel/conv/conv3d.rs	45.76%	96 Missing ⚠️
crates/burn-autodiff/src/ops/module.rs	69.43%	81 Missing ⚠️
crates/burn-tensor/src/tensor/ops/modules/conv.rs	88.64%	46 Missing ⚠️
crates/burn-tch/src/ops/module.rs	0.00%	43 Missing ⚠️
crates/burn-candle/src/ops/module.rs	0.00%	14 Missing ⚠️
crates/burn-import/src/onnx/op_configuration.rs	90.97%	12 Missing ⚠️
crates/burn-core/src/nn/padding.rs	61.90%	8 Missing ⚠️
crates/burn-core/src/record/serde/adapter.rs	25.00%	6 Missing ⚠️
crates/burn-ndarray/src/ops/conv.rs	98.54%	4 Missing ⚠️
crates/burn-tensor/src/repr/operation.rs	91.30%	4 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1945      +/-   ##
==========================================
+ Coverage   85.04%   85.40%   +0.36%     
==========================================
  Files         793      805      +12     
  Lines       94600    99248    +4648     
==========================================
+ Hits        80449    84760    +4311     
- Misses      14151    14488     +337

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

antimora · 2024-07-01T22:13:03Z

Can we add a couple check boxes under: Conv3d and ConvTranspose3d in https://github.com/tracel-ai/burn/blob/main/crates/burn-import/SUPPORTED-ONNX-OPS.md? Folks would be happy to see we now support Conv2d and ConvTranspose3d!

laggui

Looked over the ONNX import, everything looks good! 👍

Thanks for adding conv3d 🙏

laggui · 2024-07-02T11:51:22Z

backend-comparison/Cargo.toml

@@ -72,10 +72,19 @@ name = "conv-transpose2d"
 path = "benches/conv_transpose2d.rs"
 harness = false

+[[bench]]


Nice! We even have a benchmark available 🔥

laggui · 2024-07-02T12:05:16Z

crates/burn-import/src/onnx/op_configuration.rs

+        match key.as_str() {
+            "kernel_shape" => kernel_shape = value.clone().into_i64s(),
+            "strides" => strides = value.clone().into_i64s(),
+            "pads" => pads = value.clone().into_i64s(),


Just realized we don't parse the auto_pad attribute (mutually exclusive) but that applies to all conv so no worries we can keep it as the current limitation. Don't think it's used that much.

booti386 · 2024-07-02T12:53:35Z

Great! Thank you all for your very positive reviews!

laggui · 2024-07-02T13:08:24Z

Great! Thank you all for your very positive reviews!

Before we merge could you update the SUPPORTED-ONNX-OPS.md file to reflect the new conv3d (as pointed out by @antimora)?

Thank you!

antimora

LGTM

antimora · 2024-07-02T15:04:35Z

A test is failing after a recent Async merge:

Since this is not related to this PR changes, @nathanielsimard @louisfd @laggui , how should we go about this?

laggui · 2024-07-02T17:09:27Z

The issue has been fixed on main.

Pretty sure the tests were all passing before the rebase so we could probably just merge, but if not sure then the fix could be merged before running the tests.

This reverts commit d696d74.

* Implement 3D and transposed 3D convolutions. * Merge changes from onnx-ir tracel-ai#1921 pr

This reverts commit d696d74.

* Implement 3D and transposed 3D convolutions. * Merge changes from onnx-ir #1921 pr --------- Co-authored-by: Dilshod Tadjibaev <939125+antimora@users.noreply.github.com>

This reverts commit 7e12168.

* Implement 3D and transposed 3D convolutions. * Merge changes from onnx-ir #1921 pr --------- Co-authored-by: Dilshod Tadjibaev <939125+antimora@users.noreply.github.com>

This reverts commit d696d74.

This reverts commit b8b47ea.

booti386 force-pushed the main branch 2 times, most recently from ba5eebd to 06bc6f2 Compare June 30, 2024 00:04

antimora requested review from laggui, nathanielsimard and louisfd June 30, 2024 16:43

louisfd approved these changes Jul 1, 2024

View reviewed changes

antimora approved these changes Jul 1, 2024

View reviewed changes

laggui approved these changes Jul 2, 2024

View reviewed changes

Implement 3D and transposed 3D convolutions.

2c67448

booti386 force-pushed the main branch from 06bc6f2 to 2c67448 Compare July 2, 2024 13:22

antimora approved these changes Jul 2, 2024

View reviewed changes

antimora added 2 commits July 2, 2024 16:01

Merge remote-tracking branch 'upstream/main' into pr/1945

32a299a

Merge changes from onnx-ir tracel-ai#1921 pr

bb0565e

antimora mentioned this pull request Jul 2, 2024

Conv3d feature request #1278

Closed

antimora merged commit d696d74 into tracel-ai:main Jul 2, 2024
6 of 12 checks passed

antimora added a commit that referenced this pull request Jul 2, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)"

7cf07a2

This reverts commit d696d74.

antimora pushed a commit to antimora/burn that referenced this pull request Jul 2, 2024

Implement 3D and transposed 3D convolutions. (tracel-ai#1945)

4f2308c

* Implement 3D and transposed 3D convolutions. * Merge changes from onnx-ir tracel-ai#1921 pr

antimora pushed a commit to antimora/burn that referenced this pull request Jul 2, 2024

Implement 3D and transposed 3D convolutions. (tracel-ai#1945)

9cb4ed7

* Implement 3D and transposed 3D convolutions. * Merge changes from onnx-ir tracel-ai#1921 pr

syl20bnr added a commit that referenced this pull request Jul 4, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)"

04877d5

This reverts commit d696d74.

syl20bnr added a commit that referenced this pull request Jul 5, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)"

75e842b

This reverts commit 7e12168.

syl20bnr added a commit that referenced this pull request Jul 5, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)" + #1961

6a09326

This reverts commit d696d74.

syl20bnr added a commit that referenced this pull request Jul 5, 2024

Main with revert of #1945 and #1961

9e976d9

nathanielsimard added a commit that referenced this pull request Jul 5, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)"

e128937

This reverts commit d696d74.

nathanielsimard added a commit that referenced this pull request Jul 5, 2024

Revert "Implement 3D and transposed 3D convolutions. (#1945)"

b8b47ea

This reverts commit d696d74.

nathanielsimard added a commit that referenced this pull request Jul 5, 2024

Revert "Revert "Implement 3D and transposed 3D convolutions. (#1945)""

954ed6b

This reverts commit b8b47ea.

antimora mentioned this pull request Jul 5, 2024

Implement 3D and transposed 3D convolutions #1978

Merged

syl20bnr pushed a commit that referenced this pull request Jul 5, 2024

Revert "Revert "Implement 3D and transposed 3D convolutions. (#1945)""

ea0db5a

This reverts commit b8b47ea.

syl20bnr pushed a commit that referenced this pull request Jul 5, 2024

Revert "Revert "Implement 3D and transposed 3D convolutions. (#1945)""

882a27c

This reverts commit b8b47ea.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement 3D and transposed 3D convolutions. #1945

Implement 3D and transposed 3D convolutions. #1945

booti386 commented Jun 29, 2024 •

edited

Loading

antimora commented Jun 30, 2024

nathanielsimard commented Jul 1, 2024

louisfd left a comment

antimora left a comment

codecov bot commented Jul 1, 2024 •

edited

Loading

antimora commented Jul 1, 2024

laggui left a comment

laggui Jul 2, 2024 •

edited

Loading

laggui Jul 2, 2024

booti386 commented Jul 2, 2024

laggui commented Jul 2, 2024

antimora left a comment

antimora commented Jul 2, 2024

laggui commented Jul 2, 2024

Implement 3D and transposed 3D convolutions. #1945

Implement 3D and transposed 3D convolutions. #1945

Conversation

booti386 commented Jun 29, 2024 • edited Loading

Checklist

Related Issues/PRs

Changes

antimora commented Jun 30, 2024

nathanielsimard commented Jul 1, 2024

louisfd left a comment

Choose a reason for hiding this comment

antimora left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 1, 2024 • edited Loading

Codecov Report

antimora commented Jul 1, 2024

laggui left a comment

Choose a reason for hiding this comment

laggui Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

laggui Jul 2, 2024

Choose a reason for hiding this comment

booti386 commented Jul 2, 2024

laggui commented Jul 2, 2024

antimora left a comment

Choose a reason for hiding this comment

antimora commented Jul 2, 2024

laggui commented Jul 2, 2024

booti386 commented Jun 29, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading

laggui Jul 2, 2024 •

edited

Loading