Conversation
| run: | | ||
| cd ./tests/code-gen | ||
| for example in matmul minimal indices slicing; do | ||
| for example in matmul minimal indices slicing bugfixing; do |
There was a problem hiding this comment.
The term "bugfixing" does not really say anything. Could you find a better name for this, please?
There was a problem hiding this comment.
In this case, it's meant to have test cases that would've failed before—i.e. bugs that were fixed (hence also the PR number where they were fixed as comments). Not sure how to name it better; maybe fixed_bugs?
There was a problem hiding this comment.
Since it is more about testing for earlier bugs, I would include something that clarifies this is testing for older bugs. Something like testing_old_bugs or a shorter alternative. I don't like what I suggested because it is too long, but you could choose something comprehensive.
| def visit_Add(self, node): | ||
| variables = [self.visit(child) for child in node] | ||
| assert len(variables) > 1 | ||
| assert len(variables) >= 1 |
There was a problem hiding this comment.
Just to clarify, could you please elaborate a bit on this change?
There was a problem hiding this comment.
The computation also works for length 1 without adjustments (but not so for 0 I think, but I haven't tried it); so all we do here is make the assert more lenient for that exact case.
(it doesn't quite return the original tensor here; but will still generate a copy-scale-add operation—though that one should be, IIRC, optimized away or merged with other operations afterwards)
There was a problem hiding this comment.
How about we verify that it does not work for 0 first? If it also works for 0, maybe you could remove the assert.
result <= Add() + term)