Merge pull request #430 from batzor/feat(math)/remove-zeros-for-spars…

…e-poly

feat(math): impl remove zeros for sparse univariate poly

tachyon/math/polynomials/univariate/BUILD.bazel

@@ -101,6 +101,7 @@ tachyon_cc_library(
         "//tachyon/base/buffer:copyable",
         "//tachyon/base/containers:adapters",
         "//tachyon/base/containers:container_util",
+        "//tachyon/base/containers:cxx20_erase",
         "//tachyon/base/json",
         "//tachyon/base/ranges:algorithm",
         "//tachyon/base/strings:string_util",

tachyon/math/polynomials/univariate/lagrange_interpolation.h

@@ -31,7 +31,7 @@ bool LagrangeInterpolate(
   }
 
   if (points.size() == 1) {
-    *ret = Poly(Coeffs({evals[0]}));
+    *ret = Poly(Coeffs({evals[0]}, true));
     return true;
   }
 
@@ -101,7 +101,7 @@ bool LagrangeInterpolate(
       coeffs_sums[0][j] += coeffs_sums[i][j];
     }
   }
-  *ret = Poly(Coeffs(std::move(coeffs_sums[0])));
+  *ret = Poly(Coeffs(std::move(coeffs_sums[0]), true));
   return true;
 }
 

tachyon/math/polynomials/univariate/univariate_dense_coefficients.h

@@ -49,15 +49,17 @@ class UnivariateDenseCoefficients {
 
   constexpr UnivariateDenseCoefficients() = default;
   constexpr explicit UnivariateDenseCoefficients(
-      const std::vector<F>& coefficients)
+      const std::vector<F>& coefficients, bool cleanup = false)
       : coefficients_(coefficients) {
+    if (cleanup) RemoveHighDegreeZeros();
     CHECK_LE(Degree(), kMaxDegree);
-    RemoveHighDegreeZeros();
   }
-  constexpr explicit UnivariateDenseCoefficients(std::vector<F>&& coefficients)
+
+  constexpr explicit UnivariateDenseCoefficients(std::vector<F>&& coefficients,
+                                                 bool cleanup = false)
       : coefficients_(std::move(coefficients)) {
+    if (cleanup) RemoveHighDegreeZeros();
     CHECK_LE(Degree(), kMaxDegree);
-    RemoveHighDegreeZeros();
   }
 
   constexpr static UnivariateDenseCoefficients Zero() {
@@ -69,8 +71,12 @@ class UnivariateDenseCoefficients {
   }
 
   constexpr static UnivariateDenseCoefficients Random(size_t degree) {
-    return UnivariateDenseCoefficients(
-        base::CreateVector(degree + 1, []() { return F::Random(); }));
+    std::vector v =
+        base::CreateVector(degree + 1, []() { return F::Random(); });
+    if (v[degree].IsZero()) {
+      v[degree] = F::One();
+    }
+    return UnivariateDenseCoefficients(std::move(v));
   }
 
   // Return dense coefficients according to the given |roots|.
@@ -189,7 +195,7 @@ class UnivariateDenseCoefficients {
         coefficients[size >> 1] *= r;
       }
     }
-    return UnivariateDenseCoefficients(std::move(coefficients));
+    return UnivariateDenseCoefficients(std::move(coefficients), true);
   }
 
   std::string ToString() const {
@@ -215,6 +221,20 @@ class UnivariateDenseCoefficients {
     return ss.str();
   }
 
+  void RemoveHighDegreeZeros() {
+    while (!IsZero()) {
+      if (coefficients_.back().IsZero()) {
+        coefficients_.pop_back();
+      } else {
+        break;
+      }
+    }
+  }
+
+  constexpr bool IsClean() const {
+    return coefficients_.size() == 0 || !coefficients_.back().IsZero();
+  }
+
  private:
   friend class internal::UnivariatePolynomialOp<
       UnivariateDenseCoefficients<F, MaxDegree>>;
@@ -263,16 +283,6 @@ class UnivariateDenseCoefficients {
                            });
   }
 
-  void RemoveHighDegreeZeros() {
-    while (!IsZero()) {
-      if (coefficients_.back().IsZero()) {
-        coefficients_.pop_back();
-      } else {
-        break;
-      }
-    }
-  }
-
   std::vector<F> coefficients_;
 };
 

tachyon/math/polynomials/univariate/univariate_dense_polynomial_unittest.cc

@@ -36,7 +36,7 @@ class UnivariateDensePolynomialTest : public FiniteFieldTest<GF7> {
 TEST_F(UnivariateDensePolynomialTest, IsZero) {
   EXPECT_TRUE(Poly().IsZero());
   EXPECT_TRUE(Poly::Zero().IsZero());
-  EXPECT_TRUE(Poly(Coeffs({GF7(0)})).IsZero());
+  EXPECT_TRUE(Poly(Coeffs({GF7(0), GF7(0)}, true)).IsZero());
   for (size_t i = 0; i < polys_.size() - 1; ++i) {
     EXPECT_FALSE(polys_[i].IsZero());
   }
@@ -296,7 +296,7 @@ TEST_F(UnivariateDensePolynomialTest, MulScalar) {
   }
 
   Poly actual = poly * scalar;
-  Poly expected(Coeffs(std::move(expected_coeffs)));
+  Poly expected(Coeffs(std::move(expected_coeffs), true));
   EXPECT_EQ(actual, expected);
   poly *= scalar;
   EXPECT_EQ(poly, expected);
@@ -344,32 +344,35 @@ TEST_F(UnivariateDensePolynomialTest, FoldEven) {
   GF7 r = GF7::Random();
   Poly folded = poly.Fold<true>(r);
   EXPECT_EQ(folded, Poly(Coeffs({r * poly[0] + poly[1], r * poly[2] + poly[3],
-                                 r * poly[4] + poly[5]})));
+                                 r * poly[4] + poly[5]},
+                                true)));
 
   GF7 r2 = GF7::Random();
   Poly folded2 = folded.Fold<true>(r2);
   EXPECT_EQ(folded2,
-            Poly(Coeffs({r2 * folded[0] + folded[1], r2 * folded[2]})));
+            Poly(Coeffs({r2 * folded[0] + folded[1], r2 * folded[2]}, true)));
 
   GF7 r3 = GF7::Random();
   Poly folded3 = folded2.Fold<true>(r3);
-  EXPECT_EQ(folded3, Poly(Coeffs({r3 * folded2[0] + folded2[1]})));
+  EXPECT_EQ(folded3, Poly(Coeffs({r3 * folded2[0] + folded2[1]}, true)));
 }
 
 TEST_F(UnivariateDensePolynomialTest, FoldOdd) {
   Poly poly = Poly::Random(kMaxDegree);
   GF7 r = GF7::Random();
   Poly folded = poly.Fold<false>(r);
   EXPECT_EQ(folded, Poly(Coeffs({poly[0] + r * poly[1], poly[2] + r * poly[3],
-                                 poly[4] + r * poly[5]})));
+                                 poly[4] + r * poly[5]},
+                                true)));
 
   GF7 r2 = GF7::Random();
   Poly folded2 = folded.Fold<false>(r2);
-  EXPECT_EQ(folded2, Poly(Coeffs({folded[0] + r2 * folded[1], folded[2]})));
+  EXPECT_EQ(folded2,
+            Poly(Coeffs({folded[0] + r2 * folded[1], folded[2]}, true)));
 
   GF7 r3 = GF7::Random();
   Poly folded3 = folded2.Fold<false>(r3);
-  EXPECT_EQ(folded3, Poly(Coeffs({folded2[0] + r3 * folded2[1]})));
+  EXPECT_EQ(folded3, Poly(Coeffs({folded2[0] + r3 * folded2[1]}, true)));
 }
 
 TEST_F(UnivariateDensePolynomialTest, Copyable) {

tachyon/math/polynomials/univariate/univariate_polynomial.h

@@ -47,9 +47,13 @@ class UnivariatePolynomial final
 
   constexpr UnivariatePolynomial() = default;
   constexpr explicit UnivariatePolynomial(const Coefficients& coefficients)
-      : coefficients_(coefficients) {}
+      : coefficients_(coefficients) {
+    DCHECK(coefficients_.IsClean()) << "Coefficients are not cleaned up.";
+  }
   constexpr explicit UnivariatePolynomial(Coefficients&& coefficients)
-      : coefficients_(std::move(coefficients)) {}
+      : coefficients_(std::move(coefficients)) {
+    DCHECK(coefficients_.IsClean()) << "Coefficients are not cleaned up.";
+  }
 
   constexpr static bool IsCoefficientForm() { return true; }
 
@@ -370,6 +374,7 @@ class RapidJsonValueConverter<math::UnivariateDensePolynomial<F, MaxDegree>> {
     math::UnivariateDenseCoefficients<F, MaxDegree> dense_coeffs;
     if (!ParseJsonElement(json_value, "coefficients", &dense_coeffs, error))
       return false;
+    dense_coeffs.RemoveHighDegreeZeros();
     *value = math::UnivariatePolynomial(std::move(dense_coeffs));
     return true;
   }

tachyon/math/polynomials/univariate/univariate_polynomial_ops.h

@@ -757,6 +757,9 @@ class UnivariatePolynomialOp<UnivariateSparseCoefficients<F, MaxDegree>> {
   }
 
   static UnivariatePolynomial<D> ToDense(const UnivariatePolynomial<S>& self) {
+    if (self.IsZero()) {
+      return UnivariatePolynomial<D>::Zero();
+    }
     size_t size = self.Degree() + 1;
     std::vector<F> coefficients(size);
     OPENMP_PARALLEL_FOR(size_t i = 0; i < size; ++i) {
@@ -823,7 +826,6 @@ class UnivariatePolynomialOp<UnivariateSparseCoefficients<F, MaxDegree>> {
     }
 
     c.coefficients_ = S(std::move(c_terms));
-    c.coefficients_.RemoveHighDegreeZeros();
   }
 
   static void DoMul(const UnivariatePolynomial<S>& a,
@@ -852,7 +854,6 @@ class UnivariatePolynomialOp<UnivariateSparseCoefficients<F, MaxDegree>> {
     }
     pdqsort(c_terms.begin(), c_terms.end());
     c.coefficients_ = S(std::move(c_terms));
-    c.coefficients_.RemoveHighDegreeZeros();
   }
 };
 

tachyon/math/polynomials/univariate/univariate_sparse_coefficients.h

@@ -20,6 +20,7 @@
 
 #include "tachyon/base/buffer/copyable.h"
 #include "tachyon/base/containers/adapters.h"
+#include "tachyon/base/containers/cxx20_erase_vector.h"
 #include "tachyon/base/json/json.h"
 #include "tachyon/base/logging.h"
 #include "tachyon/base/optional.h"
@@ -102,15 +103,16 @@ class UnivariateSparseCoefficients {
 
   constexpr UnivariateSparseCoefficients() = default;
   constexpr explicit UnivariateSparseCoefficients(
-      const std::vector<Term>& terms)
+      const std::vector<Term>& terms, bool cleanup = false)
       : terms_(terms) {
+    if (cleanup) RemoveZeros();
     CHECK_LE(Degree(), kMaxDegree);
-    RemoveHighDegreeZeros();
   }
-  constexpr explicit UnivariateSparseCoefficients(std::vector<Term>&& terms)
+  constexpr explicit UnivariateSparseCoefficients(std::vector<Term>&& terms,
+                                                  bool cleanup = false)
       : terms_(std::move(terms)) {
+    if (cleanup) RemoveZeros();
     CHECK_LE(Degree(), kMaxDegree);
-    RemoveHighDegreeZeros();
   }
 
   constexpr static UnivariateSparseCoefficients CreateChecked(
@@ -157,6 +159,15 @@ class UnivariateSparseCoefficients {
     return !operator==(other);
   }
 
+  constexpr bool IsClean() const {
+    for (const Term& term : terms_) {
+      if (term.coefficient.IsZero()) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   constexpr F& at(size_t i) {
     F* ptr = const_cast<F*>(std::as_const(*this).GetCoefficient(i));
     CHECK(ptr);
@@ -273,6 +284,10 @@ class UnivariateSparseCoefficients {
     return ss.str();
   }
 
+  void RemoveZeros() {
+    base::EraseIf(terms_, [](const Term& x) { return x.coefficient.IsZero(); });
+  }
+
  private:
   friend class internal::UnivariatePolynomialOp<
       UnivariateDenseCoefficients<F, MaxDegree>>;
@@ -289,16 +304,6 @@ class UnivariateSparseCoefficients {
     return &it->coefficient;
   }
 
-  void RemoveHighDegreeZeros() {  // Fix to RemoveZeros
-    while (!IsZero()) {
-      if (terms_.back().coefficient.IsZero()) {
-        terms_.pop_back();
-      } else {
-        break;
-      }
-    }
-  }
-
   std::vector<Term> terms_;
 };
 

tachyon/math/polynomials/univariate/univariate_sparse_polynomial_unittest.cc

@@ -39,7 +39,8 @@ class UnivariateSparsePolynomialTest : public FiniteFieldTest<GF7> {
 TEST_F(UnivariateSparsePolynomialTest, IsZero) {
   EXPECT_TRUE(Poly().IsZero());
   EXPECT_TRUE(Poly::Zero().IsZero());
-  EXPECT_TRUE(Poly(Coeffs({{0, GF7(0)}})).IsZero());
+  EXPECT_TRUE(Poly(Coeffs({{0, GF7(0)}, {1, GF7(0)}}, true)).IsZero());
+
   for (size_t i = 0; i < polys_.size() - 1; ++i) {
     EXPECT_FALSE(polys_[i].IsZero());
   }
@@ -324,7 +325,7 @@ TEST_F(UnivariateSparsePolynomialTest, MulScalar) {
   }
 
   Poly actual = poly * scalar;
-  Poly expected(Coeffs(std::move(expected_terms)));
+  Poly expected(Coeffs(std::move(expected_terms), true));
   EXPECT_EQ(actual, expected);
   poly *= scalar;
   EXPECT_EQ(poly, expected);

tachyon/zk/plonk/vanishing/vanishing_prover_impl.h

@@ -175,7 +175,7 @@ void VanishingProver<Poly, Evals, ExtendedPoly, ExtendedEvals>::BatchEvaluate(
       coeffs[j] += h_pieces[i][j];
     }
   }
-  combined_h_poly_ = Poly(Coefficients(std::move(coeffs)));
+  combined_h_poly_ = Poly(Coefficients(std::move(coeffs), true));
 
   prover->EvaluateAndWriteToProof(random_poly_.poly(), x);
 }