Fix BlockDiag failure to compile in numba backend #1830
Conversation
ricardoV94
added
bug
Removes depracated `create_tuple_creator`
ricardoV94
force-pushed
the
block_diag_bug
branch
3 times, most recently
from
37fae8f to
fb308ca
Compare
ricardoV94
changed the title
jessegrabowski
left a comment
jessegrabowski
left a comment
There was a problem hiding this comment.
Approved with a small request for change.
| out[r : r + rr, c : c + cc] = arr | ||
| r += rr | ||
| c += cc | ||
| rr, cc = arr0.shape |
There was a problem hiding this comment.
Do we miss out on any LLVM optimizations by eagerly in-lining the loop? They're supposed to be good at loops.
There was a problem hiding this comment.
You cannot loop through mixed read-only/regular arrays, as you can't put them in a container that numba will be happy about? Also I think numba is good at iterating over arrays, not containers of them.
There was a problem hiding this comment.
I thought it was supposed to be good at loops in general
There was a problem hiding this comment.
All claims I see (and what I expect from what they're doing) is about loops on arrays
| @register_funcify_default_op_cache_key(BlockDiagonal) | ||
| def numba_funcify_BlockDiagonal(op, node, **kwargs): | ||
| dtype = node.outputs[0].dtype | ||
| """Codegen something like: |
There was a problem hiding this comment.
Add a note why codegen is required here (the readonly problem)
There was a problem hiding this comment.
The note is time-context sensitive. You could argue a note is then needed in all the Ops that require codegen. The common theme is anything with variadic arguments basically needs it: Alloc, Scan, OFG, Elemwise (that uses low level) ...
There was a problem hiding this comment.
Yeah then we should do that in each one of those ops. Or write some docs, heaven forbid.
There was a problem hiding this comment.
docs get stale, code is docs, long live LLM-auto-real-time-doc
There was a problem hiding this comment.
us: "why doesn't anyone want to help develop pytensor"
also us: "lol docs? figure it out idiot"
| return f"({', '.join(x)})" | ||
|
|
||
|
|
||
| class CODE_TOKEN(Enum): |
There was a problem hiding this comment.
oo a wild Enum. Make me feel all warm and Java-y
ricardoV94
force-pushed
the
block_diag_bug
branch
from
fb308ca to
b28a98d
Compare
2 participants
Fix failure in BlockDiag
Numba is so limited for meta-programming, that we often have only two options:
This PR adds a super simple helper for string codegen, so we can build statements programatically, and it handles the indentation. It's super cumbersome otherwise, and since string codegen isn't going anywhere I think it's worth it. I tried to keep it really barebones.