lightning-thunder icon indicating copy to clipboard operation
lightning-thunder copied to clipboard
verified publisher icon Lightning-AI

adding stride_order before convolution

Open jjsjann123 opened this issue 1 year ago • 1 comments

Before submitting
  • [ ] Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • [ ] Did you read the contributor guideline, Pull Request section?
  • [ ] Did you make sure to update the docs?
  • [ ] Did you write any new necessary tests?

What does this PR do?

Fixes # (issue).

PR review

Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

jjsjann123 avatar Apr 10 '24 09:04 jjsjann123

numerical issue looks a bit annoying. Changing the torch reference implementation using channels last. I thought this is automatically done in cudnn (NOT VERIFIED!), but there's still places where torch convolution goes through native kernels.

jjsjann123 avatar Apr 10 '24 19:04 jjsjann123

@jjsjann123 what's your view on this? The CI seemed to pass earlier, but the PR is still a draft and thus not merged...

t-vi avatar May 29 '24 11:05 t-vi

sorry about keeping this one floating. I haven't got any chance to revist its impact on vision models, so I wasn't confidently pushing for it.

I'll close it for now and revisit when I have a bit more bandwidth.

jjsjann123 avatar May 31 '24 16:05 jjsjann123