This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse][taco] Support true dense tensors and all dense sparse tensors.
ClosedPublic

Authored by bixia on Feb 10 2022, 4:13 PM.

Details

Summary

The only method to create a true dense tensor (i.e un-annotated) in MLIR-PyTACO
is through the from_array method. However, the annotated all dense tensors are
also implemented as true dense tensor currently. The PR fixes the
implementation to support annotated all dense sparse tensors.

Extend the tensor init method to support the construction of a tensor without
any sparsity annotation.

Change the tensor to_file method to only support writing unpacked sparse
tensors to file through the MLIR sparse tensor dialect.

Add unit tests for true dense tensors and all dense sparse tensors.

Diff Detail

Event Timeline

bixia created this revision.Feb 10 2022, 4:13 PM
bixia requested review of this revision.Feb 10 2022, 4:13 PM

I found the summary a bit confusion at first reading, because some information appears later in the text.
For historical accuracy I would rewrite this a bit, as in

Currently the only method to create a true dense tensor (i.e un-annotated) in MLIR-PyTACO is through
the from_array method. All other dense tensors in PyTACO are implemented as tensor with all dense
dimensions. This PR changes .....

Also, I am pondering a bit if we want to keep the ability to define tensors with all dense dimensions as well,
just like we can at MLIR level. Probably not important for this revision, so we can chat a bit more about it later.

mlir/test/Integration/Dialect/SparseTensor/taco/test_simple_tensor_algebra.py
33

Our tests often give a lot of output with e.g. date and times, so just checking for "2" is very brittle.
I would make the output more precise e.g.

print('Number of passed:', passed)

and check for that

Per off line explanation, None vs all-dense. Please add a test case test_Dense.py that shows all different ways explicitly

bixia updated this revision to Diff 408074.Feb 11 2022, 3:12 PM
bixia marked an inline comment as done.

Address the review comment by: revising the summary; printing more information for FileCheck.

bixia added a comment.Feb 11 2022, 3:13 PM

I rewrote the commit message. PTAL.

Can you please also a pytaco test for the new feature? Both to test, but also to document in a way.

bixia updated this revision to Diff 408493.Feb 14 2022, 10:29 AM

Add a PyTACO test to show the use of true dense tensors.

bixia updated this revision to Diff 408494.Feb 14 2022, 10:32 AM

Fix the comment in a test.

bixia updated this revision to Diff 408513.Feb 14 2022, 10:56 AM

Fix a comment.

aartbik added inline comments.Feb 14 2022, 11:29 AM
mlir/test/Integration/Dialect/SparseTensor/taco/test_true_dense_tensor_algebra.py
11–12

These two lines are not used, right?

bixia updated this revision to Diff 408608.Feb 14 2022, 1:55 PM
bixia marked an inline comment as done.

Address review comments: remove two unused lines, add a comment for is_dense=True.

aartbik accepted this revision.Feb 14 2022, 2:18 PM

Ship it!

This revision is now accepted and ready to land.Feb 14 2022, 2:18 PM
bixia retitled this revision from [mlir][sparse][taco] Fix the definition of dense tensors. to [mlir][sparse][taco] Support true dense tensors and all dense sparse tensors..Feb 14 2022, 2:54 PM
bixia edited the summary of this revision. (Show Details)
bixia updated this revision to Diff 408638.Feb 14 2022, 3:10 PM

Rebase.

This revision was landed with ongoing or failed builds.Feb 14 2022, 3:35 PM
This revision was automatically updated to reflect the committed changes.