Opened 3 years ago

Closed 2 years ago

#17282 closed enhancement (fixed)

Implementing Wehler K3 Surfaces

Reported by: jdefaria Owned by:
Priority: major Milestone: sage-6.8
Component: algebraic geometry Keywords:
Cc: Merged in:
Authors: Joao Alberto de Faria Reviewers: Ben Hutz, Grayson Jorgenson
Report Upstream: N/A Work issues:
Branch: 08877a4 (Commits) Commit: 08877a497294020d4b090daba982560fe2b5b4be
Dependencies: Stopgaps:

Description

Adding code for Wehler K3 surfaces, along with some useful tests regarding their properties

Change History (35)

comment:1 Changed 3 years ago by jdefaria

  • Branch set to u/jdefaria/ticket/17282
  • Created changed from 11/03/14 15:42:40 to 11/03/14 15:42:40
  • Modified changed from 11/03/14 15:42:40 to 11/03/14 15:42:40

comment:2 Changed 3 years ago by git

  • Commit set to 3b7005d3330aed23b09eed04ff82037d3b7caf7b

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

3b7005dAdded Wk3 file

comment:3 Changed 3 years ago by git

  • Commit changed from 3b7005d3330aed23b09eed04ff82037d3b7caf7b to c263b79aae1c28982248f7548ba65fe20b5fae1e

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

c263b79Finished fixing documenation

comment:4 Changed 3 years ago by jdefaria

  • Authors changed from Joao de Faria to Joao Alberto de Faria
  • Milestone changed from sage-6.4 to sage-6.6
  • Status changed from new to needs_review

comment:5 Changed 3 years ago by git

  • Commit changed from c263b79aae1c28982248f7548ba65fe20b5fae1e to 5e79368ba628a26e2ef0ca0ac62f48edfefcfe37

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

5e79368Fixed formating in documentation

comment:6 Changed 3 years ago by git

  • Commit changed from 5e79368ba628a26e2ef0ca0ac62f48edfefcfe37 to 9b268f823c0e0ff35f0b881dadaa50ca72bfe875

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

9b268f8Added references and fixed long lines

comment:7 Changed 3 years ago by bhutz

  • Reviewers set to Ben Hutz, Grayson Jorgenson

Started reviewing. Done through degenerate_fibers(). Will continue later.

From docbuild:

  • top of file todo block not formatted correctly
  • degenerate_fibers() - instead of
  • is_degenerate() - # lines
  • is_isomorphic() - other -> right
  • lambda_plus() - examples not formatted correctly
  • sigmax() - P2 x 'P2 should not have '
  • orbit_phi(), orbit_psi() - input not formatted correctly

From code:

  • WehlerK3Surface: check that polys is a list or tuple
  • randomWehlerK3:
    • replace getmon with xmrange([3,3])
    • Why can we choose L to be the diagonal bilinear polynomial

WehlerK3Surface_ring:

  • init():
    • get rid of ZZ(2),ZZ(2) - need to change ProductProjectiveSpace_init
    • assert all(x.parent() is ZZ for x in N) to N=[Integer(x) for x in N]
    • move TypeError? check line 142 to line 135
  • _morphism(): delete since it just inherits from parent
  • change_ring() check not needed
  • Sxa: condense the 4 lines

AS = self.ambient_space() ASC = AS.coordinate_ring() PSY = AS[1]; PSYC = PSY.coordinate_ring()

to

PSY = self.ambient_space()[1]

  • Lx*, Qx*, Sx* - need to check that a is a point in P2
  • Ramification_poly():
    • need to wrap return
    • need to put in definition in documentation
  • is_degenerate(): delete comments line 675-677
  • degenerate_fibers():
    • line 772-794: should use rational_points on subschemes from ticket 18008
    • line 803-825: should use rational_points on subschemes from ticket 18008
  • degenerate_primes(): document the algorithm
    • rename: badPrimes
    • todo - over number fields

comment:8 Changed 2 years ago by bhutz

Review continues. All that is left after this block is the review the sigmaX and sigmaY for degenerate fibers

is_smooth():

  • Algorithm statement: is not a sentence
  • remove extra spaces (line 940,941)

sigmaX:

  • put doc string on new line (line 964). Better description in docstring
  • [CaSi?] does not enmpty lines around it
  • Input: Point on surface in P2 \times P2
  • make check like call in projective morphism
  • 1024: can just be else instead of elif
  • remove extra spaces 987-988
  • remove s= from degenerate examples

sigmaY:

  • Better description in docstring
  • [CaSi?] does not enmpty lines around it
  • Input: Point on surface in P2 \times P2
  • make check like call in projective morphism
  • remove extra spaces 1170-1171
  • remove s= from degenerate examples
  • 1205: else instead of elif

phi:

  • [CaSi?] does not enmpty lines around it
  • Input: Point on surface in P2 \times P2
  • 1362: the point coordatines should be a list.
  • remove extra spaces 1358-1359
  • check can be false on the 2nd call

psi:

  • white space around check = True
  • [CaSi?] does not enmpty lines around it
  • Input: Point on surface in P2 \times P2
  • 1362: the point coordatines should be a list.
  • remove extra spaces 1394-1395
  • check can be false on the 2nd call

lamba_plus:

  • [CaSi?] citation in description
  • give a description of the algorithm
  • P - a surface point
  • remove extra spaces 1434-1435
  • 1452-1453, 1455-1456 - less indendation
  • rename localHeight - no CamelCase
  • 1472-1477 use GPoly(1,l) do not need the ifs. Same for all of the 'if j ==' (1479 - 1496 -> one line)
  • should the *Q[0][i]/Q[0][k] be PK not Q (as in 1508)
  • reduce 1500-1524 to one line
  • add some comments
  • is 1529 using newQ so that the coordinates don't blow up??

lamba_minus:

  • all the same comments

canonical_height_plus:

  • [CaSi?] citation in description
  • give a description of the algorithm
  • P - a surface point
  • 1699: missing [] around coordinates
  • 1708: one less indent

canonical_height_minus:

  • all the same comments

canonical_height:

  • better description
  • give a description of the algorithm
  • P - a surface point
  • remove some extra spaces in examples
  • 1817 - need [] around coordinate points

Fibers:

  • The name of the function Fibers shouldn't be plural
  • Fix the description
  • input should be a P2 (not a list) - so then also fix 1859-1863
  • comment on why you have 'and' in the is_square
  • citation (Hutz - Thesis)
  • case 4.2.2 is missing in 1915-1918
  • 1923 remove comment
  • combine checking for duplicates with detereming the fiber (i.e., check if in Fibers before adding
  • rename Fibers to fiber
  • 1940 remove this comment

orbit_phi:

  • remove 1947
  • input needs a blank line after and has a terminating white space that should be removed
  • fix description of N (see projective_point.orbit)
  • another example where N is a list [m,n]
  • can't always normalize such as in CC. Should you add a keyword as is projective_point.orbit?

orbit_psi:

  • all the same comments

is_isomorphic:

  • add reasoning to the description
  • add a False example
  • fix example spacing

is_symmetric_orbit:

  • blank line befor function
  • better function description
  • fixing spacing 2075-2076
  • this could be given a list of random points. You need to also check the each succesive is the image of the previous one
  • actually you just be given a random list and the function would still run...

FiberCounts?:

  • no camelcase for name
  • make the name more descriptive of the output
  • desribe algorithm (citation)
    • actually this is just straight up enumeration
    • add a TODO to use the more advanced counting (citation)
  • 2134 - less indent
  • 2137 needs another space around +

comment:9 Changed 2 years ago by bhutz

  • Status changed from needs_review to needs_work

Here is the last bit, reviewing the degenerate fiber morphisms:

sigamX:

  • 1034-35, 1047-48, 1061-62: are all exactly the same, so can be before the ifs
  • 1036, 1037 - switch order so that t = w1-t1 (and in the other 2 places)
  • put the T after the ifs since they are all the same
  • 1078: use .divides
  • 1076-1088: needs some comments
  • 1116 use divides
  • The whole degenerate fiber computation is badly in need of comments/explanation

all the same comments for sigmaY

comment:10 Changed 2 years ago by git

  • Commit changed from 9b268f823c0e0ff35f0b881dadaa50ca72bfe875 to b987250835ecba1acc43c020b8f4d5f314adfaf8

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

b987250-17282- Fixed issues laid out in review

comment:11 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:12 Changed 2 years ago by bhutz

  • Status changed from needs_review to needs_work

There are a couple issues from the above lists that were not fixed and there are a few more things I found:

  • is_isomorphic() - other -> right
  • sigmax() - P2 x 'P2 should not have '
  • Lx*, Qx*, Sx* - need to check that a is a point in P2
    • it would be better to check that it is the 'correct' projective space, self.ambient_space()[*]
  • Ramification_poly() definition: yes that is the formula for it, but what is it?
  • degenerate_primes: Fix the formatting for the default value. (True default) -> Default: True
    • check for other instances in the file
  • some comments in lambda_plus, lambda_minus wouldn't hurt
  • trailing white space in fiber()
  • nth_iterate_phi, nth_iterate_psi
    • p -> P
    • n an integer (not a positive integer)
  • is_symmetric_orbit: you don't just want a list of points, you want a periodic cycle...
  • normalize parameter for phi, psi, sigmaX, sigmaY, orbit_phi, orbit_psi, nth_iterate_phi, nth_iterate_psi
    • use self.normalize_coordinates() to normalize (inherited from projective product)
    • when these functions are called and the base_ring is a field it should normalize in the same way init does (make the last nonzero coordinate 1)
PP.<x0,x1,x2,y0,y1,y2> = ProductProjectiveSpaces([2,2],GF(3))
L = x0*y0 + x1*y1 - x2*y2
Q = x0*x1*y1^2 + x2^2*y0*y2
W = WehlerK3Surface([L,Q])
H=End(W)
f=H([x0,x1,x2,y0,y1,y2])
  • the _morphism() function is not needed since it will inherit it from its ambient space (projective product)
  • The coordinates of the points need to be coerced into the base_ring
    PP.<x0,x1,x2,y0,y1,y2> = ProductProjectiveSpaces([2,2],GF(3))
    L = x0*y0 + x1*y1 - x2*y2
    Q = x0*x1*y1^2 + x2^2*y0*y2
    W = WehlerK3Surface([L,Q])
    P=W.point([0,2,0,2,0,0],False);P
    P.normalize_coordinates();P
    

This may actually be an issue from projective products.

  • I have not yet run the docbuild, but I noticed a number of typos in the documentation, you should probably take a close look at the html version to catch those.

comment:13 Changed 2 years ago by git

  • Commit changed from b987250835ecba1acc43c020b8f4d5f314adfaf8 to 26bff005e05b2a151baceb1134d9c50d8f160ec2

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

94943baMerge branch 'master' into ticket/17282
26bff00Merged into 6.7 beta1 and made minor changes

comment:14 Changed 2 years ago by git

  • Commit changed from 26bff005e05b2a151baceb1134d9c50d8f160ec2 to 092c8cd9095248afc60cda1382b920d9ace99688

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

092c8cd-17282- Adjusted problems from last review

comment:15 Changed 2 years ago by git

  • Commit changed from 092c8cd9095248afc60cda1382b920d9ace99688 to 33c3109a3856aa468da5d2fb0fc14aaf9b62283e

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

33c3109-17282- Fixed documentation issues and commented code

comment:16 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:17 Changed 2 years ago by bhutz

  • Milestone changed from sage-6.6 to sage-6.7
  • Status changed from needs_review to needs_work

Getting closer so I did a more thorough functionality testing:

  • line 2224 note that it is an example of an "asymmetric 8-cycle with points on degenerate fibers"
  • degenerate_fibers seems to be failing for Qp
R.<x0,x1,x2,y0,y1,y2>=!PolynomialRing(Qp(3, 300),6)
Y=x0*y0+x1*y1-x2*y2
Z=x0!^2*y0*y1 + x0!^2*y2!^2 - x0*x1*y1*y2 + x1!^2*y2*y1 +x2!^2*y2!^2 + x2!^2*y1!^2 +x1!^2*y2!^2
X=WehlerK3Surface([Z,Y])
print X.is_degenerate()
X.degenerate_fibers()

  • Here is another interesting example you can include
R.<x0,x1,x2,y0,y1,y2>=!PolynomialRing(QQ,6)
L= (-y0 - y1)*x0 + (-y0*x1 - y2*x2)
Q=(-y2*y0 - y1!^2)*x0!^2 + ((-y0!^2 - y2*y0 + (-y2*y1 - y2!^2))*x1 + (-y0!^2 - y2*y1)*x2)*x0 + ((-y0!^2 - y2*y0 - y2!^2)*x1!^2 + (-y2*y0 - y1!^2)*x2*x1 + (-y0!^2 + (-y1 - y2)*y0)*x2!^2)
X=WehlerK3Surface([L,Q])
P=X([1,0,-1,1,-1,0]) #order 16
X.nth_iterate_phi(P,8) == X.nth_iterate_psi(P,8)

  • here is a failure in sigmaY (degenerate)
PP.<x0,x1,x2,y0,y1,y2>=!ProductProjectiveSpaces([2,2],GF(3))
l=x0*y0 + x1*y1 + x2*y2
q=-3*x0!^2*y0!^2 + 4*x0*x1*y0!^2 - 3*x0*x2*y0!^2 - 5*x0!^2*y0*y1 - 190*x0*x1*y0*y1 - 5*x1!^2*y0*y1 + 5*x0*x2*y0*y1 + 14*x1*x2*y0*y1 + 5*x2!^2*y0*y1 - x0!^2*y1!^2 - 6*x0*x1*y1!^2 - 2*x1!^2*y1!^2 + 2*x0*x2*y1!^2 - 4*x2!^2*y1!^2 + 4*x0!^2*y0*y2 - x1!^2*y0*y2 + 3*x0*x2*y0*y2 + 6*x1*x2*y0*y2 - 6*x0!^2*y1*y2 - 4*x0*x1*y1*y2 - x1!^2*y1*y2 + 51*x0*x2*y1*y2 - 7*x1*x2*y1*y2 - 9*x2!^2*y1*y2 - x0!^2*y2!^2 - 4*x0*x1*y2!^2 + 4*x1!^2*y2!^2 - x0*x2*y2!^2 + 13*x1*x2*y2!^2 - x2!^2*y2!^2
X=WehlerK3Surface([l,q])
print X.degenerate_fibers()
P=X([0,1,1,1,0,0])
print P
print X.sigmaY(P)
X.sigmaY(X.sigmaY(P))

TypeError: Must be ZZ or QQ

  • you have a couple --long test failures due to a lack of
set_verbose(None)

comment:18 Changed 2 years ago by git

  • Commit changed from 33c3109a3856aa468da5d2fb0fc14aaf9b62283e to 1edfe71b0fda3685fd3edc0bd9b9e1532d549f28

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

1edfe71-17282- Fixed the issues found in last pass

comment:19 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

I've gone through and fixed the sigmaY issue as well as the long test failures, unfortunately p-adic factoring for polynomials is not working properly now, so that error is unavoidable, setting to needs review once more

comment:20 Changed 2 years ago by bhutz

  • Status changed from needs_review to needs_work

Here is what I see in the documentation:

ramification poly

  • cite call silverman
  • output: polynomial in coordinate ring of ambient space

canonical height

  • change '-' to 'minus'

canonical height minus

  • change '+' to 'minus'
  • 'Use v=0 for the archimedean place' should be deleted
  • add (local heights) after 'lambda minus heights'

canonical height plus

  • - 'Use v=0 for the archimedean place' should be deleted
  • add (local heights) after 'lambda plus heights'

degenerate fibers

  • Remove the extra 'in' in 'The output is a list of lists where in the elements of lists are points in the appropriate projective space.'

degenerate primes

  • only over ZZ or QQ

is_symmetric_oribt

  • fix 'ps`'

lambda minus

  • change '-' to 'minus'

lambda plus

  • change '+' to 'plus'

nth_iterate_phi

  • write out 'phi'
  • quotes for `psi' is fine, but you should be consistent throughout the

documentation. This is thefirst place you've done that.

nth_iterate_phi

  • write out 'psi'

orbit_phi

  • Function is defined in [CaSi]

phi

  • output: a point on self

psi

  • output: a point on self

sigmaX

  • ``self`` - you have single quotes
  • change 'while we have two points in the fiber' to 'while we can split the fiber into pairs of points'
  • output: a point on self

sigmaY

  • ``self`` - you have single quotes
  • change 'while we have two points in the fiber' to 'while we can split the fiber into pairs of points'
  • output: a point on self

you are inconsistent with the hyphenation of bilinear and biquadratic. I think it

should not be hyphenated.


For functionality

fiber:

  • add a component = 0 example
  • shouldn't the last 3 cases have an else for component == 1?

PP.<x0,x1,x2,y0,y1,y2> = ProductProjectiveSpaces([2,2],GF(7)) L = x0*y0 + x1*y1 - 1*x2*y2 Q=(2*x0^2 + x2*x0 + (2*x1^2 + x2^2))*y0^2 + ((x0^2 + x1*x0 +(x1^2 + 2*x2*x1 + x2^2))*y1 + (2*x1^2 + x2*x1 + x2^2)*y2)*y0 + ((2*x0^2+ (x1 + 2*x2)*x0 + (2*x1^2 + x2*x1))*y1^2 + ((2*x1 + 2*x2)*x0 + (x1^2 +x2*x1 + 2*x2^2))*y2*y1 + (2*x0^2 + x1*x0 + (2*x1^2 + x2^2))*y2^2) W = WehlerK3Surface([L,Q]) W.fiber([4,0,1],0)

Should have 2 points...


PP.<x0,x1,x2,y0,y1,y2> = ProductProjectiveSpaces([2,2],GF(7)) L = x0*y0 + x1*y1 - 1*x2*y2 Q = (2*y0^2 + y2*y0 + (2*y1^2 + y2^2))*x0^2 + ((y0^2 + y1*y0 +(y1^2 + 2*y2*y1 + y2^2))*x1 + (2*y1^2 + y2*y1 + y2^2)*x2)*x0 + ((2*y0^2+ (y1 + 2*y2)*y0 + (2*y1^2 + y2*y1))*x1^2 + ((2*y1 + 2*y2)*y0 + (y1^2 +y2*y1 + 2*y2^2))*x2*x1 + (2*y0^2 + y1*y0 + (2*y1^2 + y2^2))*x2^2) W = WehlerK3Surface([L,Q]) W.fiber([1,0,0],1)

This is returning a point over base field with a entry in a quadratic extension. That is 2 issues.


nth_iterates

  • Unless I'm missing something, the negative seems not to be working

R.<x0,x1,x2,y0,y1,y2>=PolynomialRing(QQ,6) L = (-y0 - y1)*x0 + (-y0*x1 - y2*x2) Q = (-y2*y0 - y1^2)*x0^2 + ((-y0^2 - y2*y0 + (-y2*y1 - y2^2))*x1 + (-y0^2 - y2*y1)*x2)*x0 + ((-y0^2 - y2*y0 - y2^2)*x1^2 + (-y2*y0 - y1^2)*x2*x1 + (-y0^2 + (-y1 - y2)*y0)*x2^2) X = WehlerK3Surface([L,Q]) P = X([1,0,-1,1,-1,0]) X.nth_iterate_phi(P,1)==X.nth_iterate_psi(P,-1)

comment:21 Changed 2 years ago by git

  • Commit changed from 1edfe71b0fda3685fd3edc0bd9b9e1532d549f28 to 728d278b2d2696a7e68a4cbc05ff1f2b323d96f0

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

728d278-17282- fixed functionality and documentation issues

comment:22 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:23 Changed 2 years ago by git

  • Commit changed from 728d278b2d2696a7e68a4cbc05ff1f2b323d96f0 to 97700cb6b9ddb04dc1c03b115246989f6adce38f

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

97700cb-17262- Fixed minor typos in documentation

comment:24 Changed 2 years ago by bhutz

  • Status changed from needs_review to positive_review

looks good now.

comment:25 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build

comment:26 Changed 2 years ago by bhutz

  • Milestone changed from sage-6.7 to sage-6.8
  • Status changed from needs_work to positive_review

I'm getting the same 82 warnings when building the reference/scheme folder as pdf on 6.7 with and without this ticket included. So the failure seems to have nothing to do with this ticket.

fyi, they are all of the form:

sage/sage-dev/src/doc/en/reference/schemes/sage/schemes/toric/weierstrass.rst:: WARNING: unusable reference target found: ../../../../../../../html/en/reference/geometry/sage/geometry/lattice_polytope.html#sage.geometry.lattice_polytope.ReflexivePolytopes

comment:27 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

I don't know what you are doing wrong but underscore is definitely not valid it TeX text mode:

! Missing $ inserted.
<inserted text> 
                $
l.28196 \bibitem[Hutz_thesis]{Hutz_thesis}
                                          {\phantomsection\label{sage/scheme...

?  [164]]
! Emergency stop.

comment:28 Changed 2 years ago by git

  • Commit changed from 97700cb6b9ddb04dc1c03b115246989f6adce38f to 930cbb023cdf849c85acf1dc67cc32274ccbe7fa

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

0bb0cf4-17282- Changed reference name
930cbb0Merge branch 'master' into ticket/17282

comment:29 Changed 2 years ago by jdefaria

I merged the ticket in with 6.7, and fixed the name of the reference to take out the underscore and now I come back with only three errors:

[schemes  ] None:None: WARNING: citation not found: Fulton
[schemes  ] /home/joao/sage-dev/src/doc/en/reference/schemes/index.rst:: WARNING: unusable reference target found: ../genindex.html
[schemes  ] /home/joao/sage-dev/src/doc/en/reference/schemes/index.rst:: WARNING: unusable reference target found: ../py-modindex.html
[schemes  ] /home/joao/sage-dev/src/doc/en/reference/schemes/index.rst:: WARNING: unusable reference target found: ../search.html

however, it still fails to build.

comment:30 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

Per sage-devel, installed tex and got the issue to resolve, docs built successfully on my machine

comment:31 Changed 2 years ago by bhutz

  • Status changed from needs_review to needs_work

I fixed my tex installation and this builds now for me as well. However, the first line of the file is used as the section heading and it is currently "AUTHORS". Change that to something meaningful like "Wehler K3 surfaces"

comment:32 Changed 2 years ago by git

  • Commit changed from 930cbb023cdf849c85acf1dc67cc32274ccbe7fa to 08877a497294020d4b090daba982560fe2b5b4be

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

08877a4-17282- fixed title

comment:33 Changed 2 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:34 Changed 2 years ago by bhutz

  • Status changed from needs_review to positive_review

New commits:

08877a4-17282- fixed title

comment:35 Changed 2 years ago by vbraun

  • Branch changed from u/jdefaria/ticket/17282 to 08877a497294020d4b090daba982560fe2b5b4be
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.