8000 Fix test_ops for tiny backend by Anish9901 · Pull Request #9302 · tinygrad/tinygrad · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Fix test_ops for tiny backend #9302

Draft
wants to merge 63 commits into
base: master
Choose a base branch
from
Draft

Fix test_ops for tiny backend #9302

wants to merge 63 commits into from
+131 −13

Conversation

Anish9901
Copy link
Contributor
@Anish9901 Anish9901 commented Feb 28, 2025 •
edited
Loading

With LLVM=1 LLVMOPT=0 TINY_BACKEND=1 python3 -m pytest -n auto test/test_ops.py

Test status on master (149 failures):

Screenshot 2025-03-01 at 1 36 03 AM

Test status on this PR (2 failure):

Screenshot 2025-03-05 at 4 04 08 AM

@github-actions GitHub Actions
Copy link
Contributor

This branch currently is behind tinygrad/master. The line count difference bot is disabled.

@chenyuxyz
Copy link
Collaborator
chenyuxyz commented Feb 28, 2025 •
edited
Loading

locked to you since if you have done more than half of these

also it needs to work correctly in principle, not just hacking to make the test pass

@chenyuxyz chenyuxyz added the bounty locked Bounty is locked to someone label Feb 28, 2025
@bhavyagada bhavyagada mentioned this pull request Feb 28, 2025
Closed
@chenyuxyz chenyuxyz mentioned this pull request Feb 28, 2025
Merged
geohot
geohot reviewed Mar 1, 2025
View reviewed changes
extra/torch_backend/backend.py Outdated
out = Tensor.avg_pool2d(self, kernel_size, stride, dilation=1, padding=padding, ceil_mode=ceil_mode, count_include_pad=count_include_pad)
return wrap(out.gradient(self, gradient=grad_out)[0])

@torch.library.impl("aten::replication_pad1d_backward", "privateuseone")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't copy paste!

@Anish9901 Anish9901 mentioned this pull request Mar 1, 2025
Merged
@Anish9901
Copy link
Contributor Author

Update: Currently there are 2 tests failing on this PR, TestOps.test_biased_conv2d and TestOps.test_pad_circular_mode

  • The conv2d test fails in the backward pass probably due to some missing condition.
  • The pad test however is quite tricky to figure out, I suspect that the issue is with the _copy_from() function.

@Anish9901 Anish9901 changed the title Fix test_ops for tiny backend(WIP) Fix test_ops for tiny backend Mar 4, 2025
@Anish9901 Anish9901 marked this pull request as ready for review March 4, 2025 22:50
@Anish9901 Anish9901 requested a review from geohot March 4, 2025 22:53
chenyuxyz
chenyuxyz reviewed Mar 4, 2025
View reviewed changes
Copy link
Collaborator
@chenyuxyz chenyuxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice progress, can you open a separate pr for simple stuff?

also functions end with _ means inplace and should have assigned

read the diff carefully and remove all unneeded changes

Anish9901 added a commit to Anish9901/tinygrad that referenced this pull request Mar 5, 2025
@Anish9901
extact functions from tinygrad#9302
aaa199e
@Anish9901 Anish9901 mentioned this pull request Mar 5, 2025
Merged
chenyuxyz added a commit that referenced this pull request Mar 5, 2025
@Anish9901 @chenyuxyz
Fix test_ops for tiny backend part 2 (#9358)
b3ac60c 
* extact functions from #9302

* revert gather and add aten.elu_backward

* address review

---------

Co-authored-by: chenyu <chenyu@fastmail.com>
@Anish9901
Copy link
Contributor Author

I have pinned down that both of the tests are failing because of the _copy_from() function, if I set the memory in dest device to be contiguous before assigning src in the cpu -> tiny step, test_biased_conv2d succeeds, it also succeeds if I comment out that line altogether. When I tried to use the aten::_to_copy function, both test succeeded, but it resulted in a failure due to recursion depth in a lot of other tests. Any suggestions on what I should look into?

@chenyuxyz
Copy link
Collaborator

pick one way to fix the test first, and enable the test on CI to show that test_ops passes in CI, then we can decide the next step

@eitanturok eitanturok mentioned this pull request Mar 6, 2025
Draft
5 tasks
@Anish9901 Anish9901 marked this pull request as draft March 7, 2025 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenyuxyz chenyuxyz chenyuxyz left review comments

@geohot geohot Awaiting requested review from geohot

Assignees
No one assigned
Labels
bounty locked Bounty is locked to someone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Anish9901 @chenyuxyz @geohot
0