Skip to content

[mlir][affine] Add unit tests for isProjectedPermutation #114775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

banach-space
Copy link
Contributor

The only way to test isProjectedPermutation is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.

@llvmbot
Copy link
Member

llvmbot commented Nov 4, 2024

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

The only way to test isProjectedPermutation is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.


Full diff: https://github.com/llvm/llvm-project/pull/114775.diff

1 Files Affected:

  • (modified) mlir/unittests/IR/AffineMapTest.cpp (+41)
diff --git a/mlir/unittests/IR/AffineMapTest.cpp b/mlir/unittests/IR/AffineMapTest.cpp
index 081afadd632f1b..2c96c69a69d203 100644
--- a/mlir/unittests/IR/AffineMapTest.cpp
+++ b/mlir/unittests/IR/AffineMapTest.cpp
@@ -21,3 +21,44 @@ TEST(AffineMapTest, inferMapFromAffineExprs) {
   map.replace(replacements);
   EXPECT_EQ(map, map);
 }
+
+TEST(AffineMapTest, isProjectedPermutation) {
+  MLIRContext ctx;
+  OpBuilder b(&ctx);
+
+  // 1. Empty map - a projected permutation.
+  AffineMap map1 = b.getEmptyAffineMap();
+  EXPECT_TRUE(map1.isProjectedPermutation());
+
+  // 2. Contains a symbol - not a projected permutation.
+  AffineMap map2 = AffineMap::get(0, 1, &ctx);
+  EXPECT_FALSE(map2.isProjectedPermutation());
+
+  // 3. The result map is {0} - since zero results are _allowed_, this _is_ a
+  // projected permutation.
+  auto zero = b.getAffineConstantExpr(0);
+  AffineMap map3 = AffineMap::get(1, 0, {zero}, &ctx);
+  EXPECT_TRUE(map3.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 4. The result map is {0} - since zero results are _not allowed_, this _is not_
+  // a projected permutation.
+  AffineMap map4 = AffineMap::get(1, 0, {zero}, &ctx);
+  EXPECT_FALSE(map4.isProjectedPermutation(/*allowZeroInResults=*/false));
+
+  // 5. The number of results > inputs, not a projected permutation.
+  AffineMap map5 = AffineMap::get(1, 0, {zero, zero}, &ctx);
+  EXPECT_FALSE(map5.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 6. A constant result that's not a {0} - not a projected permutation.
+  auto one = b.getAffineConstantExpr(1);
+  AffineMap map6 = AffineMap::get(1, 0, {one}, &ctx);
+  EXPECT_FALSE(map6.isProjectedPermutation(/*allowZeroInResults=*/true));
+
+  // 7. Not a dim expression - not a projected permutation.
+  auto d0 = b.getAffineDimExpr(0);
+  auto d1 = b.getAffineDimExpr(1);
+
+  auto sum = d0 + d1;
+  AffineMap map7 = AffineMap::get(2, 0, {sum}, &ctx);
+  EXPECT_FALSE(map7.isProjectedPermutation());
+}

Copy link

github-actions bot commented Nov 4, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

The only way to test `isProjectedPermutation` is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.
@banach-space banach-space force-pushed the andrzej/add_unit_tests_for_map branch from dc8ada1 to 9f03639 Compare November 4, 2024 15:17
Copy link
Contributor

@javedabsar1 javedabsar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. ProjectedPermutation is used in many places and these are very instructive tests.

MLIRContext ctx;
OpBuilder b(&ctx);

// 1. Empty map - a projected permutation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a 'projected permutation' test and TRUE, FALSE tells the expected so no need to repeat 'a projected permutation'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, thanks!

// 7. Not a dim expression - not a projected permutation.
auto d0 = b.getAffineDimExpr(0);
auto d1 = b.getAffineDimExpr(1);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some tougher tests
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d0,d1,d2,d4)
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d0+d1,d2,d4)
(d0,d1, d2, d3,d4,d5) ->(d5,d3,d2,d4)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, thanks, about to add these.

@banach-space banach-space force-pushed the andrzej/add_unit_tests_for_map branch from f06eb81 to ef83b59 Compare November 5, 2024 13:53
Copy link
Contributor

@javedabsar1 javedabsar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now. Thanks.

@banach-space banach-space merged commit 8b7af60 into llvm:main Nov 5, 2024
8 checks passed
@banach-space banach-space deleted the andrzej/add_unit_tests_for_map branch November 5, 2024 16:54
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
The only way to test `isProjectedPermutation` is through unit tests. The
concept of "projected permutations" is tricky to document and these
tests are a good source documentation of the expected/intended
behavoiur. Hence these additional unit tests.
banach-space added a commit to banach-space/llvm-project that referenced this pull request Nov 20, 2024
The only way to test `getInversePermutation` is through unit tests. The
concept of "inverse permutations" is tricky to document and these tests
are a good source documentation of the expected/intended behavoiur.
Hence these additional unit tests.

This is a follon-on llvm#114775 for in which I added tests for
`isProjectedPermutation`.
banach-space added a commit that referenced this pull request Nov 22, 2024
The only way to test `getInversePermutation` is through unit tests. The
concept of "inverse permutations" is tricky to document and these tests
are a good source documentation of the expected/intended behavoiur.
Hence these additional unit tests.

This is a follow-on of #114775 in which I added tests for
`isProjectedPermutation`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants