Opened 11 months ago

Closed 9 months ago

#20895 closed enhancement (fixed)

Computing ordinary models of plane curves

Reported by: gjorgenson Owned by:
Priority: minor Milestone: sage-7.4
Component: algebraic geometry Keywords: gsoc2016
Cc: bhutz, mmarco Merged in:
Authors: Grayson Jorgenson Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 6ef31f5 (Commits) Commit: 6ef31f5e81344759cc6a27941cd30ace24f6de9f
Dependencies: Stopgaps:


Given a plane curve, it is possible to transform it into a plane curve with only ordinary singularities via application of a finite sequence of quadratic transformation maps.

Implement a function at the curve class level to apply the standard quadratic transformation (the birational automorphism of P2 sending (x : y : z) to (yz : xz : xy)) and a function to transform a given plane curve into one with only ordinary singularities.

Change History (18)

comment:1 Changed 11 months ago by gjorgenson

  • Branch set to u/gjorgenson/ticket/20895

comment:2 Changed 11 months ago by git

  • Commit set to 54a991e606b87e665bef1ea53c229f5a5df265f8

Branch pushed to git repo; I updated commit sha1. New commits:

489c55f20895: some pieces of functionality
f6cca2520895: merge with ticket 20839.
54a991e20895: first pass at implementation

comment:3 Changed 11 months ago by gjorgenson

Okay, here's my first attempt at the implementation. I improved the affine tangents function to work with QQbar, and added some helper functions to apply the standard Cremona transformation, find the equation of the line between two projective plane points, and to move a curve into excellent position as defined in Fulton's alg. curves book. I haven't yet been able to find a good way to mitigate the dependency on QQbar, so right now I just restrict to QQbar curves entirely. Moving curves to excellent position seems to take > 2 seconds even for basic curves and quickly becomes impractical for more complicated curves. The new functionality does seem to be working correctly on the basic examples I've tested though.

Do you think it's possible to reduce the dependency on QQbar computations?

comment:4 Changed 11 months ago by mmarco

Take a look at the .as_number_field_element() method of algebraic numbers, and the number_field_elements_from_algebraicsfunction. They allow you to get a number field to embed your numbers. So you can proceed this way: use QQbar just to compute the roots you need. Once done that, find a number field where everything you need may fit (that is, it should be an extension of your current field that contains the needed elements), and extend your base ring. From there, you can keep working in a concrete number field which should be much faster than QQbar.

comment:5 Changed 11 months ago by git

  • Commit changed from 54a991e606b87e665bef1ea53c229f5a5df265f8 to 1c91bf1b7fa839e488e23306acf6dbf78720b77f

Branch pushed to git repo; I updated commit sha1. New commits:

1c91bf120895: some revisions

comment:6 Changed 11 months ago by git

  • Commit changed from 1c91bf1b7fa839e488e23306acf6dbf78720b77f to 132dfb58a02856fb2650d17330866def4ddbb175

Branch pushed to git repo; I updated commit sha1. New commits:

132dfb520895: more efficient implementation

comment:7 Changed 11 months ago by git

  • Commit changed from 132dfb58a02856fb2650d17330866def4ddbb175 to dd402019c59ce2889d22933d3167079f0faf5997

Branch pushed to git repo; I updated commit sha1. New commits:

dd4020120895: ordinary_model revision, and some minor fixes

comment:8 Changed 11 months ago by gjorgenson

Alright, I experimented with embedding into numberfields, but after doing some timing analysis, I found that the biggest use of time was finding intersection points of a given curve with the lines used to create a change of coordinates map to move the curve into excellent position. I tried revising the excellent_position and ordinary_model functions in the first of the last three commits to reduce the costs of these computations, but they still used a lot of time even for simple examples.

In the last two commits I tried a different approach and gave the excellent_position function an option to accept a list/tuple of three points to use to create the transformation (without checks), and modified the ordinary model function so that it now creates lists of vertices incrementally without explicitly checking that they put the curve into excellent position and passes them to excellent_position. To verify that the nonordinary singularities do become resolved, I'm using that after every application of excellent_position + quadratic_transformation, if the given curve was actually put into excellent position, the resulting curve should either have a smaller apparent genus (arithmetic genus - sum m*(m-1)/2 as m runs over the curve's singular point multiplicities), or should have fewer nonordinary singularities. This gives an upper bound for the number of applications of excellent_position + quadratic_transformation needed to resolve the nonordinary singularities, and so if the nonordinary singularities are not resolved after this number of transformations, a new set of vertices is used.

So far this seems to work pretty quickly for curves of degree < 5, but is somewhat hit-or-miss for higher degree curves. The transformed curves can also have high degrees when multiple transformations are needed to resolve all of the nonordinary singularities, and sometimes don't seem very practically useful. I think the code is a bit too much of a mess right now for this to be ready for review, but does the method of implementation seem okay so far? Do you think there's a way I can make the transformations nicer?

comment:9 Changed 11 months ago by git

  • Commit changed from dd402019c59ce2889d22933d3167079f0faf5997 to 837ddb8b01cb421e7568545f8f2181c29a47a389

Branch pushed to git repo; I updated commit sha1. New commits:

ce8c06a20895: revised excellent_position and ordinary_model
329941820895: merge with #20848 for access to is_irreducible
837ddb820895: minor fix in excellent_position and added is_irreducible check

comment:10 Changed 11 months ago by gjorgenson

  • Status changed from new to needs_review

I implemented a new approach for excellent_position that no longer requires computing intersection points and works for number fields. I think the computations are faster now and I revised ordinary_model to leave the checks to excellent_position. ordinary_model works for number fields as well by returning a curve defined over an extension if any of the coordinates of the non-ordinary singularities are not contained in the original base field.

I also cleaned up the code and removed unnecessary changes that were implemented in previous commits, such as the line function for projective space.

comment:11 Changed 9 months ago by git

  • Commit changed from 837ddb8b01cb421e7568545f8f2181c29a47a389 to 71bd9a091149b640499c1d168845035b39fd0dcc

Branch pushed to git repo; I updated commit sha1. New commits:

1811424Merge branch 'master' into u/gjorgenson/ticket/20895
b1a505dMerge branch 'master' into u/gjorgenson/ticket/20895
a405aaf20895: some minor improvements
71bd9a020895: bug and documentation fix

comment:12 Changed 9 months ago by gjorgenson

  • Milestone changed from sage-7.3 to sage-7.4

I made some improvements to the functionality here and I think this should be ready for review.

Changes made in this ticket up to this point have been to add an ordinary_model function along with two helper functions, quadratic_transform and excellent_position, to the projective plane curve class. The is_ordinary_singularity and tangents functions for affine curves were also modified to reduce the need for QQbar computations.

Overall the ordinary_model function appears to be working properly, but still becomes very slow for most curves of degree > 5. I think this slowdown is mainly due to the rate at which the degrees of the quadratic transforms of the curves can grow.

comment:13 Changed 9 months ago by bhutz

  • Reviewers set to Ben Hutz
  • Status changed from needs_review to needs_work

I did not find any functionality issues, but here are some comments

  • in is there a reason you have
    from sage.arith.misc import binomial

in the function and not with the rest of the imports

  • in affine tangents()
t =[0])
for monom in T.monomials():
    if[0]) < t:
        t =[0]

seems to just be doing

min([e1 for e1,e2 in T.exponents()])

is that right? If so, you can simplify both instances.

  • in is_ord : projective
C = self.affine_patch(i)
Q = list(P)
t = Q.pop(i)
Q = [1/t*Q[j] for j in range(self.ambient_space().dimension_relative())]

You can just say


to get the affine point

  • in excellent position: projective
d = self.defining_polynomial().degree()

is there no function for plane curves??

if all([[0]) > 0 for g in T.monomials()]):

seems simpler as

if all([e[0] > 0 for e in T.exponents()]):

also this function fails over QQbar since .divides() does not work. You should either fix that or specify in the documentation that you need a number field

  • in def ordinary_model(self):
Return an ordinary plane curve model of this curve.

Actually you are returning the morphism from the curve to an ordinary model. You should either match the description of excellent position or return the curve and add a return_mapping parameter. Either way, I think both functions should do the same thing.

I'd also like an example to show that all singularities are now ordinary.

[C.is_ordinary_singularity(Q) for Q in C.singular_points()]

in extension()

not sure why you are doing this:

# make sure the defining polynomial variable names are the same for K, N
N = NumberField(K.defining_polynomial().parent()(F.defining_polynomial()), str(K.gen()))


sage: K.<v>=QuadraticField(2)
sage: N = NumberField(K.defining_polynomial().parent()(x^3+2), str(K.gen()))
sage: N.gen()==K.gen()

comment:14 Changed 9 months ago by git

  • Commit changed from 71bd9a091149b640499c1d168845035b39fd0dcc to 6ef31f5e81344759cc6a27941cd30ace24f6de9f

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

982851420790: merge with 7.3 beta5
ee363e420790: merge with 7.3 beta6
c69bcd120790: removed constructor imports
16d620720790: merge with sage 7.3.rc0
878503220790: changes from review
39108a120790: added some tests
67320bbMerge branch 'master' into u/gjorgenson/ticket/20790
679338b20790: merge with 21085 to fix conflicts
ea2970f20895: merge with ticket 20790 to fix conflicts
6ef31f520895: fix example

comment:15 Changed 9 months ago by gjorgenson

  • Status changed from needs_work to needs_review

Thanks for reviewing this. I was a bit busy with preparing for the semester and wasn't able to finish working on the next update until now.

I made the suggested changes and also merged this with ticket 20790 (has been closed) to fix a conflict with the imports. The projective subscheme multiplicity and intersection_multiplicity functions now use the point dehomogenize function as well for cleaner code. I also added a degree function override for projective plane curves which just returns the degree of the defining polynomial of the curve to avoid using the slower Hilbert polynomial computation.


N = NumberField(K.defining_polynomial().parent()(F.defining_polynomial()), str(K.gen()))

I don't think str(K.gen()) is needed and can be replaced with an arbitrary variable name, but the reason I am using K.defining_polynomial().parent()(F.defining_polynomial()) is to make the defining polynomials of N and K have the same variable name. If the names are different the composite_fields function raises an error:

R.<x> = QQ[]
S.<y> = QQ[]
N.<a> = NumberField(x^2 + 1)
M.<b> = NumberField(y^2 - 7)
Traceback (click to the left of this block for traceback)
sage.libs.pari.handle_error.PariError: inconsistent variables in
polcompositum, x != y

comment:16 Changed 9 months ago by bhutz

  • Status changed from needs_review to positive_review

The updates look fine to me and all tests still pass.

comment:17 Changed 9 months ago by chapoton


comment:18 Changed 9 months ago by vbraun

  • Branch changed from u/gjorgenson/ticket/20895 to 6ef31f5e81344759cc6a27941cd30ace24f6de9f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.