This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Add Matricized Tensor Times Khatri-Rao Product (MTTKRP) integration test
ClosedPublic

Authored by gussmith23 on Jun 16 2021, 12:54 PM.

Diff Detail

Event Timeline

gussmith23 created this revision.Jun 16 2021, 12:54 PM
gussmith23 requested review of this revision.Jun 16 2021, 12:54 PM
aartbik added inline comments.Jun 16 2021, 1:14 PM
mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_mttkrp.mlir
46

even though "mlir" code does not has a style guideline, try to avoid 80 col overwrap (here and below). Not a hard rule, just easier to read in browsers.

72

Here and below, period at end of comment

mlir/test/Integration/data/mttkrp_b.tns
19

The double ## makes sense since we have a comment inside a comment, but is there a better way to make this jump out a bit more?

Also, we have the style guideline to end each comment with a period (full sentence).
This is of course corner case, since you show other code, but perhaps just add the .
to satisfy the trigger response on this ;-)

gussmith23 marked 2 inline comments as done.

Address Aart's comments: line length, add periods, make code comment stand out

mlir/test/Integration/data/mttkrp_b.tns
19

I tried adding >s. Let me know if that makes it any more clear, otherwise I'll try something else!

gussmith23 marked an inline comment as not done.Jun 16 2021, 2:10 PM
aartbik accepted this revision.Jun 16 2021, 5:26 PM
aartbik added inline comments.
mlir/test/Integration/data/mttkrp_b.tns
19

yeah. looks good. thanks!

This revision is now accepted and ready to land.Jun 16 2021, 5:26 PM
aartbik retitled this revision from Add Matricized Tensor Times Khatri-Rao Product (MTTKRP) integration test to [mlir][sparse] Add Matricized Tensor Times Khatri-Rao Product (MTTKRP) integration test.Jun 16 2021, 5:29 PM

Oh, in this and the previous commit, I forgot to say that we usually add tags to the title of a revision (typically [mlir] followed by a sub-component).
I usually add [mlir][sparse] for anything related to sparse tensor compilation. I have added this to the title for this revision.