luminal icon indicating copy to clipboard operation
luminal copied to clipboard

allow noop tensor `expand`

Open swfsql opened this issue 1 year ago • 3 comments

Example related to #64

swfsql avatar Jun 17 '24 17:06 swfsql

I'm re-opening because there is a simpler test, and also because if the PR is closed no new commits appear on it (which makes sense).

The added test is a R1<1> -> R1<1> (noop) expansion, and the test fails with:

assertion `left == right` failed
  left: ShapeTracker { dims: [1], indexes: [0], fake: [false], mask: [(0, 2147483647)], padding: [(0, 0)] }
 right: ShapeTracker { dims: [1, 1], indexes: [1, 0], fake: [false, true], mask: [(0, 2147483647), (0, 2147483647)], padding: [(0, 0), (0, 0)] }

Indicating that the noop expansion had changed the shape (undesired).

swfsql avatar Jun 20 '24 01:06 swfsql

I added a check on src == dst during shape expansion, and made it stop the actual shape modification in case it is the same. But although the small test now passes, I still have some shape error in another test which is not a minimal example, so I'll keep trying to reduce or notice what is the error

swfsql avatar Jun 20 '24 01:06 swfsql

@swfsql I've just pushed a really large change which moves away from const generic shapes and to only using ShapeTracker runtime shape tracking. That means this PR is probably not nessecary anymore since we don't have a .expand() anymore, it's now .expand(axis, size).

I'm still interested in const generic shape tracking, but currently rust doesn't have enough support for const generic expressions, which made the code very complex (as you're seeing) and some models unrepresentable. Once better support on the rust langauge level is stablized, we can bring it back in.

jafioti avatar Jul 08 '24 16:07 jafioti