Page MenuHomePhabricator

[mlir][sparse] Add optional start_value to sparse_tensor.init

Authored by jim22k on May 24 2022, 2:58 PM.



The start value will be used during reductions instead of assuming that
the initial value is zero, allowing e.g. multiplication during
a reduction which needs 1.0 as the starting value rather than 0.0.

Diff Detail

Event Timeline

jim22k created this revision.May 24 2022, 2:58 PM
jim22k requested review of this revision.May 24 2022, 2:58 PM

I plan to add custom reduction using sparse_tensor.binary later. This PR will focus solely on explicit starting values for non-custom reductions.


I am unhappy with the need to specify the type of startValue. It should always be the same as the result's element type. However, I tried several iterations of TypesMatchWith trait and custom builders and couldn't get it to work. I think it is because startValue is optional, so the inferring of type is tricky.

Requiring the type to be specified works, but if someone knows how to eliminate this, I think it would look better.

aartbik added inline comments.May 26 2022, 5:17 PM

replace "the" before unitialized with "an"


Perhaps give a small motivation for this


I can't think of an easy way around either (others?).

But I think it is okay, MLIR is not really a programming language in the sense that our syntax needs to be as concise as possible. Sometimes we over state the types.


this will need an invalid.mlir case

also, the new syntax needs an roundtrip.mlir

Note that there is this parallel effort to unify our tensor initialization ops (see
So let's brainstorm offline to come up with another solution.

jim22k abandoned this revision.Thu, Jun 2, 10:19 AM

With the removal of sparse_tensor.init operation, this change no longer makes sense.

@aartbik and I discussed an alternative to create a new sparse_tensor.reduce operation which will accept a start value.