Skip to content

[NFC][IR2Vec] Increasing tolerance in approximatelyEquals() of Embedding #145117

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

Open
wants to merge 1 commit into
base: users/svkeerthy/06-12-simplifying_creation_of_embedder
Choose a base branch
from

Conversation

svkeerthy
Copy link
Contributor

@svkeerthy svkeerthy commented Jun 20, 2025

Increase the default tolerance for approximatelyEquals in IR2Vec's Embedding class from 1e-6 to 1e-4.

(Tracking issue - #141817)

@svkeerthy svkeerthy changed the title Increasing tolerance in ApproximatelyEquals [NFC][IR2Vec] Increasing tolerance in approximatelyEquals() of Embedding Jun 20, 2025
@svkeerthy svkeerthy requested a review from mtrofin June 20, 2025 23:32
@svkeerthy svkeerthy marked this pull request as ready for review June 20, 2025 23:33
@llvmbot llvmbot added mlgo llvm:analysis Includes value tracking, cost tables and constant folding labels Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-mlgo

Author: S. VenkataKeerthy (svkeerthy)

Changes

Increase the default tolerance for approximatelyEquals in IR2Vec's Embedding class from 1e-6 to 1e-4.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IR2Vec.h (+1-1)
  • (modified) llvm/unittests/Analysis/IR2VecTest.cpp (+2-2)
diff --git a/llvm/include/llvm/Analysis/IR2Vec.h b/llvm/include/llvm/Analysis/IR2Vec.h
index 06312562060aa..480b834077b86 100644
--- a/llvm/include/llvm/Analysis/IR2Vec.h
+++ b/llvm/include/llvm/Analysis/IR2Vec.h
@@ -116,7 +116,7 @@ struct Embedding {
 
   /// Returns true if the embedding is approximately equal to the RHS embedding
   /// within the specified tolerance.
-  bool approximatelyEquals(const Embedding &RHS, double Tolerance = 1e-6) const;
+  bool approximatelyEquals(const Embedding &RHS, double Tolerance = 1e-4) const;
 
   void print(raw_ostream &OS) const;
 };
diff --git a/llvm/unittests/Analysis/IR2VecTest.cpp b/llvm/unittests/Analysis/IR2VecTest.cpp
index 05af55b59323b..33ac16828eb6c 100644
--- a/llvm/unittests/Analysis/IR2VecTest.cpp
+++ b/llvm/unittests/Analysis/IR2VecTest.cpp
@@ -154,14 +154,14 @@ TEST(EmbeddingTest, ApproximatelyEqual) {
   EXPECT_TRUE(E1.approximatelyEquals(E2)); // Diff = 1e-7
 
   Embedding E3 = {1.00002, 2.00002, 3.00002}; // Diff = 2e-5
-  EXPECT_FALSE(E1.approximatelyEquals(E3));
+  EXPECT_FALSE(E1.approximatelyEquals(E3, 1e-6));
   EXPECT_TRUE(E1.approximatelyEquals(E3, 3e-5));
 
   Embedding E_clearly_within = {1.0000005, 2.0000005, 3.0000005}; // Diff = 5e-7
   EXPECT_TRUE(E1.approximatelyEquals(E_clearly_within));
 
   Embedding E_clearly_outside = {1.00001, 2.00001, 3.00001}; // Diff = 1e-5
-  EXPECT_FALSE(E1.approximatelyEquals(E_clearly_outside));
+  EXPECT_FALSE(E1.approximatelyEquals(E_clearly_outside, 1e-6));
 
   Embedding E4 = {1.0, 2.0, 3.5}; // Large diff
   EXPECT_FALSE(E1.approximatelyEquals(E4, 0.01));

Copy link
Member

@mtrofin mtrofin left a comment

Choose a reason for hiding this comment

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

why - as in, expected because upcoming change; or flakyness?

@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-12-simplifying_creation_of_embedder branch from 8b8932b to 29ebe35 Compare June 23, 2025 21:10
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-20-increasing_tolerance_in_approximatelyequals branch from d05856c to bf89c59 Compare June 23, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding mlgo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants