From aee14f01754453cc5b9962e885089d7d9ce95aed Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Sat, 18 Jan 2025 19:21:14 +0100 Subject: [PATCH 1/9] Improve speed of `saturation` The function `remove` seems to be faster than `Singular.saturation`. --- src/Rings/mpoly-ideals.jl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index 6258c5efcf7..5e313f5c166 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -347,6 +347,11 @@ 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` seems to be faster than `Singular.saturation` + if ngens(I) == 1 & ngens(J) == 1 + return remove(I[1], J[1])[2] + end + if iteration K, _ = Singular.saturation(singular_generators(I), singular_generators(J)) else @@ -401,6 +406,12 @@ 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` seems to be faster than `Singular.saturation` + if ngens(I) == 1 & ngens(J) == 1 + pair = remove(I[1], J[1]) + return (pair[2], pair[1]) + end + K, k = Singular.saturation(singular_generators(I), singular_generators(J)) return (MPolyIdeal(base_ring(I), K), k) end From c5bff32ec6e07619e5e73774af47c3a13bbbd518 Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Sat, 18 Jan 2025 19:58:23 +0100 Subject: [PATCH 2/9] Fix output type --- src/Rings/mpoly-ideals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index 5e313f5c166..a6d910421e0 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -349,7 +349,7 @@ 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` seems to be faster than `Singular.saturation` if ngens(I) == 1 & ngens(J) == 1 - return remove(I[1], J[1])[2] + return ideal(remove(I[1], J[1])[2]) end if iteration @@ -409,7 +409,7 @@ function saturation_with_index(I::MPolyIdeal{T}, J::MPolyIdeal{T} = ideal(base_r # `remove` seems to be faster than `Singular.saturation` if ngens(I) == 1 & ngens(J) == 1 pair = remove(I[1], J[1]) - return (pair[2], pair[1]) + return (ideal(pair[2]), pair[1]) end K, k = Singular.saturation(singular_generators(I), singular_generators(J)) From 3296d51ea6fbc633c7e7c35513ad079b3fecf663 Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Sat, 18 Jan 2025 21:37:02 +0100 Subject: [PATCH 3/9] Fix usage of `&` --- src/Rings/mpoly-ideals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index a6d910421e0..6d426fb3b43 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -348,7 +348,7 @@ 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` seems to be faster than `Singular.saturation` - if ngens(I) == 1 & ngens(J) == 1 + if (ngens(I) == 1) & (ngens(J) == 1) return ideal(remove(I[1], J[1])[2]) end @@ -407,7 +407,7 @@ 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` seems to be faster than `Singular.saturation` - if ngens(I) == 1 & ngens(J) == 1 + if (ngens(I) == 1) & (ngens(J) == 1) pair = remove(I[1], J[1]) return (ideal(pair[2]), pair[1]) end From 83dabd4ecf554b7d43eadebe32d6ca1247f2da9e Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Mon, 20 Jan 2025 09:26:00 +0900 Subject: [PATCH 4/9] Only for saturation with respect to a variable --- src/Rings/mpoly-ideals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index 6d426fb3b43..eee1cc8d667 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -348,7 +348,7 @@ 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` seems to be faster than `Singular.saturation` - if (ngens(I) == 1) & (ngens(J) == 1) + if ((ngens(I) == 1) & (ngens(J) == 1)) && (J[1] in gens(base_ring(I))) return ideal(remove(I[1], J[1])[2]) end @@ -407,7 +407,7 @@ 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` seems to be faster than `Singular.saturation` - if (ngens(I) == 1) & (ngens(J) == 1) + if ((ngens(I) == 1) & (ngens(J) == 1)) && (J[1] in gens(base_ring(I))) pair = remove(I[1], J[1]) return (ideal(pair[2]), pair[1]) end From eefc2ce9f2c284785b34dbb01657338a20bf8f04 Mon Sep 17 00:00:00 2001 From: Erik Paemurru <143521159+paemurru@users.noreply.github.com> Date: Tue, 21 Jan 2025 20:53:28 +0900 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: ederc --- src/Rings/mpoly-ideals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index eee1cc8d667..bbac3674c3f 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -348,7 +348,7 @@ 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` seems to be faster than `Singular.saturation` - if ((ngens(I) == 1) & (ngens(J) == 1)) && (J[1] in gens(base_ring(I))) + if ((ngens(I) == 1) & (ngens(J) == 1)) return ideal(remove(I[1], J[1])[2]) end @@ -407,7 +407,7 @@ 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` seems to be faster than `Singular.saturation` - if ((ngens(I) == 1) & (ngens(J) == 1)) && (J[1] in gens(base_ring(I))) + if ((ngens(I) == 1) & (ngens(J) == 1)) pair = remove(I[1], J[1]) return (ideal(pair[2]), pair[1]) end From b33a52f26eae7f880bcd9edd8358b82d773c35d1 Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Wed, 22 Jan 2025 07:57:09 +0900 Subject: [PATCH 6/9] Remove extra parentheses --- src/Rings/mpoly-ideals.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index bbac3674c3f..6d426fb3b43 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -348,7 +348,7 @@ 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` seems to be faster than `Singular.saturation` - if ((ngens(I) == 1) & (ngens(J) == 1)) + if (ngens(I) == 1) & (ngens(J) == 1) return ideal(remove(I[1], J[1])[2]) end @@ -407,7 +407,7 @@ 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` seems to be faster than `Singular.saturation` - if ((ngens(I) == 1) & (ngens(J) == 1)) + if (ngens(I) == 1) & (ngens(J) == 1) pair = remove(I[1], J[1]) return (ideal(pair[2]), pair[1]) end From 535e36359c0d1381523276fee63b94e130c7964b Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Wed, 22 Jan 2025 21:49:29 +0900 Subject: [PATCH 7/9] Do not `remove` if unit ideal --- src/Rings/mpoly-ideals.jl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index 6d426fb3b43..57f18f8d740 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -347,8 +347,10 @@ 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` seems to be faster than `Singular.saturation` - if (ngens(I) == 1) & (ngens(J) == 1) + # `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 @@ -406,10 +408,11 @@ 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` seems to be faster than `Singular.saturation` - if (ngens(I) == 1) & (ngens(J) == 1) - pair = remove(I[1], J[1]) - return (ideal(pair[2]), pair[1]) + # `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)) + return ideal(remove(I[1], J[1])[2]) end K, k = Singular.saturation(singular_generators(I), singular_generators(J)) From 994c0fa4850f28d1a0bc58ba1aafa674a59588e8 Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Wed, 22 Jan 2025 22:07:19 +0900 Subject: [PATCH 8/9] Fix returning a pair --- src/Rings/mpoly-ideals.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rings/mpoly-ideals.jl b/src/Rings/mpoly-ideals.jl index 57f18f8d740..8b1264d8b99 100644 --- a/src/Rings/mpoly-ideals.jl +++ b/src/Rings/mpoly-ideals.jl @@ -412,7 +412,8 @@ function saturation_with_index(I::MPolyIdeal{T}, J::MPolyIdeal{T} = ideal(base_r # respect to principal ideals if ngens(I) == 1 && ngens(J) == 1 is_unit(J[1]) && return (I, base_ring(I)(0)) - return ideal(remove(I[1], J[1])[2]) + pair = remove(I[1], J[1]) + return (ideal(pair[2]), pair[1]) end K, k = Singular.saturation(singular_generators(I), singular_generators(J)) From 010dee75ff4c21fbeb9e49d9d529004ef6b1b04b Mon Sep 17 00:00:00 2001 From: Erik Paemurru Date: Thu, 23 Jan 2025 21:19:32 +0900 Subject: [PATCH 9/9] Fixing a book test --- .../brandhorst-zach-fibration-hopping/vinberg_2.jlcon | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/book/specialized/brandhorst-zach-fibration-hopping/vinberg_2.jlcon b/test/book/specialized/brandhorst-zach-fibration-hopping/vinberg_2.jlcon index 376a1d089a9..f844d16e5f6 100644 --- 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]