Parametrizing rational plane curves #846

wdecker · 2021-11-30T17:47:29Z

No description provided.

tthsqe12 · 2021-12-01T11:56:28Z

experimental/PlaneCurve/ParaPlaneCurves.jl

+
+julia> RNC = ProjCurve(I)
+Projective curve defined by the ideal(v*x - w^2, v*y - w*x, w*y - x^2, v*z - w*y, w*z - x*y, x*z - y^2)
+


The extra space here occurs because println is used here. Both should just be print.

Oscar.jl/experimental/PlaneCurve/ProjCurve.jl

Lines 43 to 48 in 45e4374

function Base.show(io::IO, C::ProjCurve)

if !get(io, :compact, false)

println(io, "Projective curve defined by the ", C.I)

else

println(io, C.I)

end

Indeed, we should address that in a later PR. There are many, many similar cases with extra trailing newlines.

@tthsqe12 thx, changed in 3da9619

fingolfin · 2021-12-01T09:49:02Z

experimental/PlaneCurve/ParaPlaneCurves.jl

-    map_to_rational_normal_curve(C::ProjPlaneCurve)
+    map_to_rational_normal_curve(C::ProjPlaneCurve{fmpq})
+
+Return a rational normal curve of degree $\deg C-2$ which `C` is mapped.


I don't understand this sentence. Is a word missing?

Suggested change

Return a rational normal curve of degree $\deg C-2$ which `C` is mapped.

Return a rational normal curve of degree $\deg C-2$ to which `C` is mapped.

Also, I'd expect a function with such a name to return a map, not the image/range of that map? (Of course that goes beyond your PR, as the function is already there and behaves this way, so I am not asking you to change it now, but perhaps this should be considered for a future edit?)

fingolfin · 2021-12-01T12:18:37Z

experimental/PlaneCurve/ParaPlaneCurves.jl

+@doc Markdown.doc"""
+    rat_normal_curve_It_Proj_Even(C::ProjCurve)
+
+Return a vector `PHI` defining an isomorphic projection of `C` to `PP^1`.


What is PP^1? A projective line? Perhaps this?

Suggested change

Return a vector `PHI` defining an isomorphic projection of `C` to `PP^1`.

Return a vector `PHI` defining an isomorphic projection of `C` to the projective line $\mathbb{P}^1`.

fingolfin · 2021-12-01T12:19:01Z

experimental/PlaneCurve/ParaPlaneCurves.jl

@@ -140,6 +258,40 @@ function rat_normal_curve_It_Proj_Odd(C::ProjCurve)
    return gens(ideal(R, J))
 end

+@doc Markdown.doc"""
+    rat_normal_curve_It_Proj_Even(C::ProjCurve)


Why is it spelled like this, with a few upper case letter in there? What does "It" mean?

fingolfin · 2021-12-01T12:20:15Z

experimental/PlaneCurve/ParaPlaneCurves.jl

+
+julia> RNC = ProjCurve(I)
+Projective curve defined by the ideal(v*x - w^2, v*y - w*x, w*y - x^2, v*z - w*y, w*z - x*y, x*z - y^2)
+


Indeed, we should address that in a later PR. There are many, many similar cases with extra trailing newlines.

fingolfin · 2021-12-01T12:21:36Z

experimental/PlaneCurve/ProjCurve.jl

@@ -39,7 +39,7 @@ mutable struct ProjCurve <: ProjectiveCurve
        n = nvars(base_ring(I)) - 1
        new(I, n, Dict{ProjCurve, ProjCurve}())
    end
-
+    


Suggested change

tthsqe12 · 2021-12-01T13:06:05Z

@wdecker The behaviour in
https://www.singular.uni-kl.de/Manual/4-2-1/sing_1712.htm#SEC1793
to get PHI is not currently replicable in oscar. Specifically, this procedure returns a new ring AND exports a global variable into the current ring: the rings used by Singular.jl are not objects into which variables can be exported. We would either have to change the caller code to try to intercept this variable, or, what is much better, change the procedure so that it actually returns PHI instead of storing it's value in a global variable that we cannot access.
Hold on, I think it is possible.

wdecker · 2021-12-01T13:51:47Z

@fingolfin thx for your comments. The functions you dicsussed are not documented in the .md file coming with this PR. Your comments will be addressed in a subseqent PR. But for this we need time: For example, we need to create data structures for rational maps.

tthsqe12 · 2021-12-01T13:52:11Z

Ok. here my changes to the function, using semi-official functions of Singular.jl (feel free to delete some comments if they are too didactic):

function rat_normal_curve_It_Proj_Even(C::ProjCurve)
    R = base_ring(C.I)
    # R is the oscar version of the original polynomial ring
    I = _tosingular_ideal(C)
    # I is a singular sideal
    L = Singular.LibParaplanecurves.rncItProjEven(I)
    # L[1] is the new ring, L[2] is its symbol table
    Rs = base_ring(I)
    # Rs is the singular version of R and is the original Singular Polynomial
    # Ring into which PHI was exported
    d = Singular.convert_ring_content(Singular.libSingular.get_ring_content(Rs.ptr), Rs)
    # d is the symbol table of Rs
    phi = d[:PHI]
    # phi is now the singular sideal whose generators we want
    O = _fromsingular_ring(L[1])
    # O is the oscar version of the new ring
    # we want to get the exported CONIC as well as the generators of PHI
    return gens(ideal(R, phi)), ProjPlaneCurve(O(L[2][:CONIC]))
end

and then using oscar,

julia> R, y =  GradedPolynomialRing(QQ, "y".*string.(1:7))
(Multivariate Polynomial Ring in 7 variables y1, y2, y3, y4, ..., y7 over Rational Field graded by 
  y1 -> [1]
  y2 -> [1]
  y3 -> [1]
  y4 -> [1]
  y5 -> [1]
  y6 -> [1]
  y7 -> [1], MPolyElem_dec{fmpq, fmpq_mpoly}[y1, y2, y3, y4, y5, y6, y7])

julia> RNC = Oscar.ProjCurve(ideal(R, [y[5]*y[6]-y[4]*y[7],
       y[4]*y[6]-y[3]*y[7],
       y[2]*y[6]-y[1]*y[7],
       y[4]*y[5]-y[2]*y[7],
       y[3]*y[5]-y[1]*y[7],
       y[1]*y[5]-y[7]^2,
       y[4]^2-y[1]*y[7],
       y[3]*y[4]-y[1]*y[6],
       y[2]*y[4]-y[1]*y[5],
       y[1]*y[4]-y[6]*y[7],
       y[2]*y[3]-y[6]*y[7],
       y[1]*y[3]-y[6]^2,
       y[2]^2-y[5]*y[7],
       y[1]*y[2]-y[4]*y[7],
       y[1]^2-y[3]*y[7],
       y[1]*y[6]^2-y[3]^2*y[7],
       y[6]^4-y[3]^3*y[7]]))
Projective curve defined by the ideal(-y4*y7 + y5*y6, -y3*y7 + y4*y6, -y1*y7 + y2*y6, -y2*y7 + y4*y5, -y1*y7 + y3*y5, y1*y5 - y7^2, -y1*y7 + y4^2, -y1*y6 + y3*y4, -y1*y5 + y2*y4, y1*y4 - y6*y7, y2*y3 - y6*y7, y1*y3 - y6^2, y2^2 - y5*y7, y1*y2 - y4*y7, y1^2 - y3*y7, y1*y6^2 - y3^2*y7, -y3^3*y7 + y6^4)


julia> Oscar.rat_normal_curve_It_Proj_Even(RNC)
(MPolyElem_dec{fmpq, fmpq_mpoly}[-y7, -y2, -y5], Projective plane curve defined by -y(1)*y(3) + y(2)^2)

fingolfin · 2021-12-02T12:26:18Z

This segfaults now in various places (see CI logs), e.g.:

Test Summary:  | Pass  Total
ParaPlaneCurve |    9      9

signal (11): Segmentation fault
in expression starting at /home/runner/work/Oscar.jl/Oscar.jl/test/Examples/PlaneCurve-test.jl:476
_Z8nlDeletePP7snumberP9n_Procs_s at /home/runner/.julia/artifacts/a24bf6445c1dac1afd4aa59f2c89f405ca3dd37e/lib/libpolys.so (unknown line)
Allocations: 904827068 (Pool: 904378289; Big: 448779); GC: 191
ERROR: Package Oscar errored during testing (received signal: 11)
Stacktrace:

tthsqe12 · 2021-12-02T12:55:42Z

Alright, we have some severe pointer ownership issues in Singular.jl. I will look into it.

tthsqe12 · 2021-12-02T15:31:17Z

@hannes14 In this code
https://github.com/oscar-system/libsingular-julia/blob/dc655e75960c9f5fb1b6e838197dddb8b8c7501b/caller.cpp#L252
does the IDDATA(h) create a fully independent copy of the data? If not, how can we create such a copy?

tthsqe12 · 2021-12-02T17:34:56Z

@wdecker This is a second try at the lookup. Incidentally, I have no idea why the first try was crashing and same mechanisms elsewhere have not crashed yet. I would add this as a suggested change, but it is not possible.

# lookup an ideal with name s in the symbol table
# TODO move this to Singular.jl
function _lookup_ideal(R::Singular.PolyRingUnion, s::Symbol)
    for i in Singular.libSingular.get_ring_content(R.ptr)
        if i[2] == s
            @assert i[1] == Singular.mapping_types_reversed[:IDEAL_CMD]
            ptr = Singular.libSingular.IDEAL_CMD_CASTER(i[3])
            ptr = Singular.libSingular.id_Copy(ptr, R.ptr)
            return Singular.sideal{elem_type(R)}(R, ptr)
        end
    end
    error("could not find PHI")
end

function rat_normal_curve_It_Proj_Even(C::ProjCurve)
    R = base_ring(C.I)
    I = _tosingular_ideal(C)
    L = Singular.LibParaplanecurves.rncItProjEven(I)
    phi = _lookup_ideal(base_ring(I), :PHI)
    O = _fromsingular_ring(L[1]::Singular.PolyRing)
    conic = L[2][:CONIC]::Singular.spoly
    return gens(ideal(R, phi)), ProjPlaneCurve(O(conic))
end

hannes14 · 2021-12-03T08:30:49Z

Am Thu, Dec 02, 2021 at 07:31:30AM -0800 schrieb dan:

@hannes14 In this code https://github.com/oscar-system/libsingular-julia/blob/dc655e75960c9f5fb1b6e838197dddb8b8c7501b/caller.cpp#L252 does the IDDATA(h) create a fully independent copy of the data? If not, how can we create such a copy?

IDDATA(h) does not copy anything (#define IDDATA(a)((a)->data.ustring)) A way to copy this pointer without several different procedures calls for all the different types would be s_internalCopy(IDTYP(h),IDDATA(h)) (https://github.com/Singular/Singular/blob/spielwiese/Singular/subexpr.cc#L430) which is unfortunatly a static procedure.

wdecker added 3 commits November 30, 2021 18:44

Parametrizing rational plane curves

705b836

corrections

8325e2b

more corrections

612e15a

tthsqe12 reviewed Dec 1, 2021

View reviewed changes

fingolfin reviewed Dec 1, 2021

View reviewed changes

replaced println by print

3da9619

wdecker added 4 commits December 1, 2021 15:03

Changed function according to Dan's suggestion

c946747

skipped line as suggested by @fingolfin

bb30af8

adjusted test to change by Dan

618afae

Another trAnother try

d3bf594

wdecker and others added 2 commits December 2, 2021 20:05

@tthsqe12 Dan's revised function

deeccbb

some tweaks

ec069e6

fingolfin enabled auto-merge (squash) December 2, 2021 23:13

fingolfin approved these changes Dec 3, 2021

View reviewed changes

fingolfin merged commit 8792057 into master Dec 3, 2021

fingolfin deleted the Wolfram branch December 3, 2021 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parametrizing rational plane curves #846

Parametrizing rational plane curves #846

wdecker commented Nov 30, 2021

tthsqe12 Dec 1, 2021

fingolfin Dec 1, 2021

wdecker Dec 1, 2021

fingolfin Dec 1, 2021

fingolfin Dec 1, 2021

fingolfin Dec 1, 2021

fingolfin Dec 1, 2021

fingolfin Dec 1, 2021

tthsqe12 commented Dec 1, 2021 •

edited

wdecker commented Dec 1, 2021

tthsqe12 commented Dec 1, 2021 •

edited

fingolfin commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

hannes14 commented Dec 3, 2021 via email


		julia> RNC = ProjCurve(I)
		Projective curve defined by the ideal(vx - w^2, vy - wx, wy - x^2, vz - wy, wz - xy, x*z - y^2)

	function Base.show(io::IO, C::ProjCurve)
	if !get(io, :compact, false)
	println(io, "Projective curve defined by the ", C.I)
	else
	println(io, C.I)
	end

	Return a rational normal curve of degree $\deg C-2$ which `C` is mapped.
	Return a rational normal curve of degree $\deg C-2$ to which `C` is mapped.

	Return a vector `PHI` defining an isomorphic projection of `C` to `PP^1`.
	Return a vector `PHI` defining an isomorphic projection of `C` to the projective line $\mathbb{P}^1`.

Parametrizing rational plane curves #846

Parametrizing rational plane curves #846

Conversation

wdecker commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tthsqe12 commented Dec 1, 2021 • edited

wdecker commented Dec 1, 2021

tthsqe12 commented Dec 1, 2021 • edited

fingolfin commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

tthsqe12 commented Dec 2, 2021

hannes14 commented Dec 3, 2021 via email

tthsqe12 commented Dec 1, 2021 •

edited

tthsqe12 commented Dec 1, 2021 •

edited