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

[svkeerthy]

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

(Tracking issue - #141817)

[svkeerthy]

• [IR2Vec] Add triplet generation utility script for vocabulary training #149215
• [IR2Vec][llvm-ir2vec] Revamp triplet generation and add entity mapping mode  #149214
• [IR2Vec][llvm-ir2vec] Add support for reading from stdin #149213
• [IR2Vec][NFC] Add helper methods for numeric ID mapping in Vocabulary #149212
• [IR2Vec] Adding documentation for llvm-ir2vec tool #148719
• [IR2Vec] Add embeddings mode to llvm-ir2vec tool #147844
• [IR2Vec] Add llvm-ir2vec tool for generating triplet embeddings #147842
• [NFC][IR2Vec] Exposing helpers in IR2Vec Vocabulary #147841
• [NFC][IR2Vec] Minor refactoring of opcode access in vocabulary #147585
• [IR2Vec] Restructuring Vocabulary #145119
• [IR2Vec] Add out-of-place arithmetic operators to Embedding class #145118
• [NFC][IR2Vec] Increasing tolerance in approximatelyEquals() of Embedding  #145117 👈 (View in Graphite)
• [IR2Vec] Simplifying creation of Embedder #143999
• [IR2Vec] Scale embeddings once in vocab analysis instead of repetitive scaling #143986
• [MLGO][IR2Vec] Integrating IR2Vec with MLInliner #143479 : 1 other dependent PR (#144139 )
• [IR2Vec] Consider only reachable BBs and non-debug instructions #143476
• [IR2Vec] Minor vocab changes and exposing weights #143200
• [IR2Vec] Exposing Embedding as an data type wrapped around std::vector<double> #143197
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@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));

[mtrofin]

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

[svkeerthy]

Mostly due to flakyness. I found that some new tests added in the upcoming PR (#145119) were failing due to the precision issues.

[svkeerthy]

Merge activity

• Jul 1, 7:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 1, 7:03 PM UTC: @svkeerthy merged this pull request with Graphite.