-
Notifications
You must be signed in to change notification settings - Fork 133
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
Improve speed of saturation
#4485
base: master
Are you sure you want to change the base?
Conversation
The function `remove` seems to be faster than `Singular.saturation`.
For one variable I can see that this helps keeping the overhead of calling Singular's |
Before the last commit, some tests did not finish (https://github.com/oscar-system/Oscar.jl/actions/runs/12854953830), so I think you are right |
Playing a bit with random examples for f and g. I get consistently a factor of at least 100x speedup of remove over saturation for principal ideals. Do not think this is just the call overhead of singular saturation. It seems impossible to me that the call overhead is 30 seconds.
Does singular catch the special case of principal ideals in its preprocessing? Can anyone produce an example where singular is faster? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After running a couple of tests I think using remove
for the principal ideal case is really worthwhile, not only if J = <x>
for a generator x
.
Still we need to fix the tests.
Co-authored-by: ederc <[email protected]>
The failing book test is just a sign difference:
But I am not sure why the other tests take too long and get cancelled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like all the long tests are timing out, getting stuck in some computation from test/AlgebraicGeometry/Schemes/EllipticSurface.jl
.
Probably the moebius transformations
test-set, this should appear after two neighbor step
but doesn't appear in the logs.
I have started that test-file locally but it is still running after 20 minutes (while on CI it takes about 5).
Sending USR1
does indeed point to a remove
call:
signal (10): User defined signal 1
_nmod_mpoly_divides_monagan_pearce at /workspace/srcdir/flint/src/nmod_mpoly/divides_monagan_pearce.c:232
nmod_mpoly_divides_monagan_pearce at /workspace/srcdir/flint/src/nmod_mpoly/divides_monagan_pearce.c:596
nmod_mpoly_divides at /workspace/srcdir/flint/src/nmod_mpoly/divides.c:192
divides at /home/lorenz/.julia/packages/Nemo/y7UoI/src/flint/nmod_mpoly.jl:483
unknown function (ip: 0x7f89d711167a)
divides at /home/lorenz/.julia/packages/Nemo/y7UoI/src/flint/fq_default_mpoly.jl:339
remove at /home/lorenz/.julia/packages/AbstractAlgebra/UzHEi/src/MPoly.jl:839
#saturation#305 at /home/lorenz/software/polymake/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:352
saturation at /home/lorenz/software/polymake/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:349
unknown function (ip: 0x7f89d69bd4e6)
saturation at /home/lorenz/software/polymake/julia/Oscar.jl/src/Rings/mpolyquo-localizations.jl:2197
_iterative_saturation at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:571
_iterative_saturation at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:565
unknown function (ip: 0x7f89c8d0ac96)
#produce_object#5672 at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:1792
produce_object at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:1696
#_#5306 at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/Sheaves.jl:52
AbsPreSheaf at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/Sheaves.jl:50 [inlined]
#5691 at ./none:0
unknown function (ip: 0x7f89c8f5a8d2)
iterate at ./generator.jl:48 [inlined]
collect_to! at ./array.jl:849
collect_to_with_first! at ./array.jl:827
unknown function (ip: 0x7f89c8f598e5)
collect at ./array.jl:801
unknown function (ip: 0x7f89c8f59392)
produce_object at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:1819
#_#5306 at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/Sheaves.jl:52
AbsPreSheaf at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/Sheaves.jl:50
unknown function (ip: 0x7f89c8f58ff2)
#colength#5764 at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:2288
colength at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Sheaves/IdealSheaves.jl:2259
unknown function (ip: 0x7f89c8d04cc6)
#intersect#5930 at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Divisors/WeilDivisor.jl:344
intersect at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/Divisors/WeilDivisor.jl:320
unknown function (ip: 0x7f89c2aa3ad6)
basis_representation at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Surfaces/EllipticSurface/EllipticSurface.jl:994
#7050 at ./none:0
unknown function (ip: 0x7f89c2aa3772)
iterate at ./generator.jl:48 [inlined]
collect at ./array.jl:791
unknown function (ip: 0x7f89c2aa13a2)
pushforward_on_algebraic_lattices at /home/lorenz/software/polymake/julia/Oscar.jl/src/AlgebraicGeometry/Surfaces/EllipticSurface/Morphisms.jl:430
unknown function (ip: 0x7f89c2a2b2c2)
jl_apply at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/julia.h:2157 [inlined]
do_call at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:126
eval_value at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:223
eval_body at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:562
eval_body at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:539
eval_body at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:539
jl_interpret_toplevel_thunk at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:821
jl_toplevel_eval_flex at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:943
jl_toplevel_eval_flex at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:886
ijl_toplevel_eval_in at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:994
eval at ./boot.jl:430 [inlined]
include_string at ./loading.jl:2734
_include at ./loading.jl:2794
include at ./Base.jl:558 [inlined]
macro expansion at ./timing.jl:581 [inlined]
#_timed_include#33 at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:5
_timed_include at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:1 [inlined]
_timed_include at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:1 [inlined]
#40 at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:258
with_unicode at /home/lorenz/.julia/packages/AbstractAlgebra/UzHEi/src/PrettyPrinting.jl:1846
#test_module#39 at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:206 [inlined]
test_module at /home/lorenz/software/polymake/julia/Oscar.jl/src/utils/tests.jl:205
unknown function (ip: 0x7f8a1538af66)
jl_apply at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/julia.h:2157 [inlined]
do_call at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:126
eval_value at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:223
eval_stmt_value at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:174 [inlined]
eval_body at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:663
jl_interpret_toplevel_thunk at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/interpreter.c:821
jl_toplevel_eval_flex at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:943
jl_toplevel_eval_flex at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:886
ijl_toplevel_eval_in at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/toplevel.c:994
eval at ./boot.jl:430 [inlined]
eval_user_input at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:245
repl_backend_loop at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:342
#start_repl_backend#59 at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:327
start_repl_backend at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:324
#run_repl#72 at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:483
run_repl at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:469
jfptr_run_repl_10104.1 at /home/lorenz/software/polymake/julia/julia/julia-1.11.2/share/julia/compiled/v1.11/REPL/u0gqU_4x0TT.so (unknown line)
#1150 at ./client.jl:446
jfptr_YY.1150_14803.1 at /home/lorenz/software/polymake/julia/julia/julia-1.11.2/share/julia/compiled/v1.11/REPL/u0gqU_4x0TT.so (unknown line)
jl_apply at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/julia.h:2157 [inlined]
jl_f__call_latest at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/builtins.c:875
#invokelatest#2 at ./essentials.jl:1055 [inlined]
invokelatest at ./essentials.jl:1052 [inlined]
run_main_repl at ./client.jl:430
repl_main at ./client.jl:567 [inlined]
_start at ./client.jl:541
jfptr__start_73406.1 at /home/lorenz/software/polymake/julia/julia/julia-1.11.2/lib/julia/sys.so (unknown line)
jl_apply at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/julia.h:2157 [inlined]
true_main at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/jlapi.c:900
jl_repl_entrypoint at /cache/build/tester-amdci5-12/julialang/julia-release-1-dot-11/src/jlapi.c:1059
main at julia-latest (unknown line)
unknown function (ip: 0x7f8a1d3e047f)
__libc_start_main at /lib64/libc.so.6 (unknown line)
unknown function (ip: 0x4010b8)
unknown function (ip: (nil))
So maybe looking into whatever ideals are generated there might produce examples where saturation is faster?
It is the function
|
Here is a minimal example of the problem:
|
For univariates we have
I will adjust it in AA to throw the same error for the multivariates. The usage here should be adjusted. |
Adjusted the code here like @thofma suggested. Assuming the tests pass, I believe this can now be merged. |
Just the signs in the booktests need to be changed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4485 +/- ##
==========================================
+ Coverage 84.39% 84.55% +0.16%
==========================================
Files 672 672
Lines 88690 88887 +197
==========================================
+ Hits 74846 75156 +310
+ Misses 13844 13731 -113
|
The function
remove
seems to be faster thanSingular.saturation
.Benchmark 1
Before:
Now:
Benchmark 2
The line
phi_star = Oscar.pushforward_on_algebraic_lattices(phi)
in the testset "moebius transformations" in the tests ofAlgebraicGeometry/Schemes/EllipticSurface
.Before:
Now: