Improve speed of saturation (#4485)

* Improve speed of `saturation` for principal ideals --------- Co-authored-by: ederc <[email protected]>
oscar-system · Jan 25, 2025 · bf8c39e · bf8c39e
1 parent f329ec4
commit bf8c39e
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 5 deletions.
diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl
@@ -347,6 +347,13 @@ Ideal generated by
 ```
 """
 function saturation(I::MPolyIdeal{T}, J::MPolyIdeal{T} = ideal(base_ring(I), gens(base_ring(I))); iteration::Bool=false) where T
+  # `remove` is faster than `Singular.saturation` when saturating with
+  # respect to principal ideals
+  if ngens(I) == 1 && ngens(J) == 1
+    is_unit(J[1]) && return I
+    return ideal(remove(I[1], J[1])[2])
+  end
+
   if iteration
     K, _ = Singular.saturation(singular_generators(I), singular_generators(J))
   else
@@ -401,6 +408,14 @@ julia> K, m = saturation_with_index(I)
 ```
 """
 function saturation_with_index(I::MPolyIdeal{T}, J::MPolyIdeal{T} = ideal(base_ring(I), gens(base_ring(I)))) where T
+  # `remove` is faster than `Singular.saturation` when saturating with
+  # respect to principal ideals
+  if ngens(I) == 1 && ngens(J) == 1
+    is_unit(J[1]) && return (I, base_ring(I)(0))
+    pair = remove(I[1], J[1])
+    return (ideal(pair[2]), pair[1])
+  end
+
   K, k = Singular.saturation(singular_generators(I), singular_generators(J))
   return (MPolyIdeal(base_ring(I), K), k)
  end

diff --git a/test/book/specialized/brandhorst-zach-fibration-hopping/vinberg_2.jlcon b/test/book/specialized/brandhorst-zach-fibration-hopping/vinberg_2.jlcon
@@ -36,11 +36,11 @@ Scheme
 with default covering
   described by patches
     1: scheme(-(x//z)^3 + (y//z)^2 - t^7 + 2*t^6 - t^5)
-    2: scheme((z//x)^3*t^7 - 2*(z//x)^3*t^6 + (z//x)^3*t^5 - (z//x)*(y//x)^2 + 1)
-    3: scheme((z//y)^3*t^7 - 2*(z//y)^3*t^6 + (z//y)^3*t^5 - (z//y) + (x//y)^3)
-    4: scheme((x//z)^3 - (y//z)^2 + s^7 - 2*s^6 + s^5)
-    5: scheme((z//x)^3*s^7 - 2*(z//x)^3*s^6 + (z//x)^3*s^5 - (z//x)*(y//x)^2 + 1)
-    6: scheme((z//y)^3*s^7 - 2*(z//y)^3*s^6 + (z//y)^3*s^5 - (z//y) + (x//y)^3)
+    2: scheme(-(z//x)^3*t^7 + 2*(z//x)^3*t^6 - (z//x)^3*t^5 + (z//x)*(y//x)^2 - 1)
+    3: scheme(-(z//y)^3*t^7 + 2*(z//y)^3*t^6 - (z//y)^3*t^5 + (z//y) - (x//y)^3)
+    4: scheme(-(x//z)^3 + (y//z)^2 - s^7 + 2*s^6 - s^5)
+    5: scheme(-(z//x)^3*s^7 + 2*(z//x)^3*s^6 - (z//x)^3*s^5 + (z//x)*(y//x)^2 - 1)
+    6: scheme(-(z//y)^3*s^7 + 2*(z//y)^3*s^6 - (z//y)^3*s^5 + (z//y) - (x//y)^3)
   in the coordinate(s)
     1: [(x//z), (y//z), t]
     2: [(z//x), (y//x), t]