One more test for rational solutions #5593

jankoboehm · 2025-11-26T10:18:37Z

This segfaults on my computer (at least when called twice), let's see what happens here.

fingolfin · 2025-11-26T10:37:07Z

Segfault in FLINT:

      From worker 4:	Starting tests for /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/experimental/OrthogonalDiscriminants/test/data.jl
      From worker 6:	Test Summary: | Pass  Total  Time
      From worker 6:	Jacobian      |    1      1  0.2s
      From worker 6:	
      From worker 6:	[444819] signal (11.1): Segmentation fault
      From worker 6:	in expression starting at /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/test/Rings/solving.jl:115
      From worker 6:	flint_mpn_copyi at /workspace/srcdir/flint-3.3.1/src/mpn_extras.h:46 [inlined]
      From worker 6:	nmod_poly_set at /workspace/srcdir/flint-3.3.1/src/nmod_poly/set.c:20
      From worker 6:	FqPolyRingElem at /home/oscarci-tester/ssd-data/ssd-runner-13/julia/packages/Nemo/kdloy/src/flint/FlintTypes.jl:4427
      From worker 6:	FqPolyRing at /home/oscarci-tester/ssd-data/ssd-runner-13/julia/packages/Nemo/kdloy/src/flint/fq_default_poly.jl:696
      From worker 6:	unknown function (ip: 0x7f98ec556875)
      From worker 6:	_jl_invoke at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:2895 [inlined]
      From worker 6:	ijl_apply_generic at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:3077
      From worker 6:	roots at /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/src/Rings/AlgClosureFp.jl:203
      From worker 6:	unknown function (ip: 0x7f98ec551545)
      From worker 6:	_jl_invoke at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:2895 [inlined]
      From worker 6:	ijl_apply_generic at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:3077
      From worker 6:	rational_solutions at /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/src/Rings/solving.jl:178
      From worker 6:	unknown function (ip: 0x7f98ec9f07b5)
      From worker 6:	_jl_invoke at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:2895 [inlined]
      From worker 6:	ijl_apply_generic at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:3077
      From worker 6:	jl_apply at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/julia.h:1982 [inlined]
      From worker 6:	do_call at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:126
      From worker 6:	eval_value at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:223
      From worker 6:	eval_body at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:489
      From worker 6:	eval_body at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:544
      From worker 6:	eval_body at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:544
      From worker 6:	jl_interpret_toplevel_thunk at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/interpreter.c:775
      From worker 6:	jl_toplevel_eval_flex at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/toplevel.c:934
      From worker 6:	jl_toplevel_eval_flex at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/toplevel.c:877
      From worker 6:	ijl_toplevel_eval_in at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/toplevel.c:985
      From worker 6:	eval at ./boot.jl:385 [inlined]
      From worker 6:	include_string at ./loading.jl:2146
      From worker 6:	_jl_invoke at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:2895 [inlined]
      From worker 6:	ijl_apply_generic at /cache/build/builder-amdci5-7/julialang/julia-release-1-dot-10/src/gf.c:3077
      From worker 4:	
      From worker 4:	GC: pause 143.40ms. collected 350.306763MB. incr 
      From worker 6:	_include at ./loading.jl:2206
      From worker 6:	include at ./Base.jl:496 [inlined]
      From worker 6:	macro expansion at ./timing.jl:503 [inlined]
      From worker 6:	#_timed_include#39 at /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/src/utils/tests.jl:5
      From worker 6:	_timed_include at /home/oscarci-tester/ssd-data/ssd-runner-13/_work/Oscar.jl/Oscar.jl/src/utils/tests.jl:1 [inlined]
...

JohnAAbbott · 2025-11-26T13:15:42Z

On my computer the first call to rational_points triggers SEGV. Why do we expect 8 points rather than 9?

fingolfin · 2025-12-03T09:00:38Z

@hannes14 did you already have a chance to look into this?

mjrodgers · 2025-12-10T10:45:36Z

from triage: @hannes14 will run this in the debugger to try to see what is going wrong

fingolfin · 2025-12-17T11:04:14Z

So the crash is originating in src/Rings/solving.jl:178:

...
      h = mapreduce(f -> evaluate(f, r), gcd, Gi)
      h == 0 && error("Unexpected zero polynomial")
      for a in roots(h)   # <-- crash is inside `roots`

Someone could insert code before the roots call to e.g. print h or store it somewhere. Then we can analyze: if we can reproduce h separately and roots(h) still crashes, then the problem is not in Singular. Or else we should be able to figure out if h somehow points to "corrupt" data.

jankoboehm · 2025-12-17T12:42:42Z

This goes through quite a number of iterations before it crashes. Of course it is not clear if something is already wrong beforehand but does not crash. The final polynomial is linear: x+31. Then the Flint exception: Unable to allocate memory.

fingolfin · 2026-01-07T10:36:57Z

I'll try to see if I can figure something out, but of course anyone else who knows a bit about FLINT and Nemo and its memory management (e.g. @thofma @lgoettgens @fieker ...) could also try... Let's see.

benlorenz · 2026-01-07T11:34:08Z

Some pointers from a debugging/rr session:

It seems to die in the conversion f = kx(b)

Oscar.jl/src/Rings/AlgClosureFp.jl

Line 203 in 56f5eac

f = kx(b)

which calls this FqPolyRingElem constructor in Nemo
https://github.com/Nemocas/Nemo.jl/blob/9067e0b89252866eb99e6fc6fa9bae68dac7a548/src/flint/FlintTypes.jl#L4475

and in then ends up in this call:

#2  nmod_poly_set (a=0x2524cc00, b=0x7f9f25cd6540) at /workspace/srcdir/flint-3.3.1/src/nmod_poly/set.c:20
20	        flint_mpn_copyi(a->coeffs, b->coeffs, b->length);
(rr) p b
$7 = (const nmod_poly_struct *) 0x7f9f25cd6540
(rr) p *b
$8 = {coeffs = 0x1, alloc = 1, length = 1, mod = {n = 1, ninv = 0, norm = 0}}

But b->coeffs is an invalid pointer which causes flint_mpn_copyi to crash.

This step from julia code (Nemo) to flint is this call:

(rr) f 0
#0  0x00007f9fabf45f0b in fq_default_poly_set_coeff (poly=0x7f9f290752d0, n=1, x=0x7f9f25cd6540, ctx=<optimized out>)
    at /workspace/srcdir/flint-3.3.1/src/fq_default_poly.h:743
743	        fq_nmod_poly_set_coeff(poly->fq_nmod, n, x->fq_nmod, FQ_DEFAULT_CTX_FQ_NMOD(ctx));
(rr) p *x
$12 = {fq = {{coeffs = 0x1, alloc = 1, length = 1}}, fq_nmod = {{coeffs = 0x1, alloc = 1, length = 1, mod = {n = 1, ninv = 0, 
        norm = 0}}}, fq_zech = {{value = 1}}, nmod = 1, fmpz_mod = {1}}

Where x->fq_nmod has a 0x1 coeffs pointer (and almost all the other entries are also just 1). This is for i=2 (n=1), i.e., the second entry in this array.

to be continued ... but maybe someone already has an idea what to look at from these bits.

benlorenz

I pushed a change that seems reasonable and does fix this for me even though I don't fully understand the details.

I pushed it here to let CI check it more thoroughly.

benlorenz · 2026-01-07T15:47:40Z

src/Rings/AlgClosureFp.jl

      push!(b, k(data(a[i])))
    elseif da[i] == l
-      push!(b, data(a[i]))
+      push!(b, k(data(a[i])))


This conversion does seem to fix the crash and seems very reasonable to me, the object returned from data might not be in the correct field for the further operations.

The conversion function (R::FqPolyRing)(x::Vector{FqFieldElem}) will only check the parent of the first element of x but they will probably not be the same without the conversion here.

nice catch!

benlorenz · 2026-01-07T15:48:26Z

test/Rings/solving.jl

+  I=ideal([x^3 + 2*y^2 + 3, y^3 + 5*x + 7])
+  pts = rational_solutions(I)
+  pts = rational_solutions(I) # this is intentional
+  @test length(pts) == 9


On my computer the first call to rational_points triggers SEGV. Why do we expect 8 points rather than 9?

Was this ever answered? Because now that it succeeds it does return 9 for me and I changed the expected output.

How about we rename the second pts to pts2, and then check that both have the same length, and the same elements (up to ordering)?

Also, one could just print the result to visually verify that the outputs are distinct. And finally, perhaps we could do something like @test all( iszero(f(p)) for p in pts, f in gens(i)) ?

Added a set comparison and iszero checks now.

jankoboehm · 2026-01-09T10:48:46Z

Great! Thanks a lot @benlorenz ! We expect 9 points. We added the 8 since the test sometimes was returning a wrong answer of 8 and sometimes segfaulting, and we wanted to distinguish this. It is very good news that now also the answer is correct!

Co-authored-by: Benjamin Lorenz <[email protected]>

One more test for rational solutions

8cc1d35

jankoboehm added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Nov 26, 2025

fingolfin requested a review from hannes14 December 3, 2025 09:00

fingolfin added triage bug: crash labels Dec 3, 2025

fingolfin removed the request for review from hannes14 January 7, 2026 10:33

benlorenz added 2 commits January 7, 2026 16:35

Merge remote-tracking branch 'origin/master' into pull/5593

95c04d1

minimize finfield: convert element to correct field

09dfdb6

benlorenz reviewed Jan 7, 2026

View reviewed changes

benlorenz mentioned this pull request Jan 8, 2026

Explicitly check in constructors taking a vector of elements (e.g. for polynomials or matrices) that the given elements have equal parents Nemocas/Nemo.jl#2220

Merged

change length of output to 9, more tests

6941ebc

benlorenz force-pushed the ratsoltest2 branch from 5628970 to 6941ebc Compare January 8, 2026 11:10

benlorenz enabled auto-merge (squash) January 9, 2026 10:52

fingolfin removed the triage label Jan 14, 2026

fingolfin approved these changes Jan 14, 2026

View reviewed changes

benlorenz merged commit 09c2cb2 into oscar-system:master Jan 14, 2026
36 checks passed

varuntrehan7 pushed a commit to varuntrehan7/Oscar.jl that referenced this pull request Jan 14, 2026

One more test for rational solutions (oscar-system#5593)

deccc59

Co-authored-by: Benjamin Lorenz <[email protected]>

One more test for rational solutions #5593

One more test for rational solutions #5593

Conversation

jankoboehm commented Nov 26, 2025

Uh oh!

fingolfin commented Nov 26, 2025

Uh oh!

JohnAAbbott commented Nov 26, 2025

Uh oh!

fingolfin commented Dec 3, 2025

Uh oh!

mjrodgers commented Dec 10, 2025

Uh oh!

fingolfin commented Dec 17, 2025

Uh oh!

jankoboehm commented Dec 17, 2025

Uh oh!

fingolfin commented Jan 7, 2026

Uh oh!

benlorenz commented Jan 7, 2026

Uh oh!

benlorenz left a comment

Choose a reason for hiding this comment

Uh oh!

benlorenz Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

fingolfin Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

benlorenz Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

fingolfin Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

benlorenz Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jankoboehm commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants