#16986 closed enhancement (fixed)
Enhance rational_preimages and all_rational_preimages to work over Number Fields
Reported by:  jdefaria  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  algebraic geometry  Keywords:  
Cc:  bhutz  Merged in:  
Authors:  Joao Alberto de Faria  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  48eebc0 (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description
Fixing the checks at the start of the code to allow both functions to work over Number Fields
Change History (22)
comment:1 Changed 4 years ago by
 Branch set to u/jdefaria/ticket/16986
 Created changed from 09/15/14 15:11:22 to 09/15/14 15:11:22
 Modified changed from 09/15/14 15:11:22 to 09/15/14 15:11:22
comment:2 Changed 4 years ago by
 Commit set to df867c1288da3a89d8a1df1d9cceb6f5dd5bacb9
 Status changed from new to needs_review
 Summary changed from Fix rational_preimages and all_rational_preimages to work over Number Fields to Enhance rational_preimages and all_rational_preimages to work over Number Fields
comment:3 Changed 4 years ago by
comment:4 Changed 4 years ago by
 Status changed from needs_review to needs_work
The cases of number fields and finite fields work fine, but many other fields fail. Also there are some issues in the documentation:
 The descriptions of both functions reference QQ only
 The description of all_rational_preimages reference parallel (which it is not)
 The text is after the :: in examples so is not formatted correctly
 number field is 2 words and not capitalized
For functionality: here are some examples which fail. They fail in at least 3 distinct ways.
R.<t>=FunctionField(QQ) P.<x,y>=ProjectiveSpace(R,1) H=End(P) f=H([x^2y^2,y^2]) f.rational_preimages(P([0,1]))
R.<t>=PolynomialRing(QQ) P.<x,y>=ProjectiveSpace(FractionField(R),1) H=End(P) f=H([x^2y^2,y^2]) f.rational_preimages(P([0,1]))
R.<t>=PolynomialRing(QuadraticField(2)) P.<x,y>=ProjectiveSpace(FractionField(R),1) H=End(P) f=H([x^2y^2,y^2]) f.rational_preimages(P([0,1]))
P.<x,y>=ProjectiveSpace(Qp(3),1) H=End(P) f=H([x^2y^2,y^2]) f.rational_preimages(P([0,1]))
P.<x,y>=ProjectiveSpace(CC,1) H=End(P) f=H([x^2y^2,y^2]) f.rational_preimages(P([0,1]))
comment:5 Changed 4 years ago by
 Reviewers set to Ben Hutz
comment:6 Changed 4 years ago by
 Commit changed from df867c1288da3a89d8a1df1d9cceb6f5dd5bacb9 to 684b502617d603bfdc5e3416aaeaed05499f6f3f
comment:7 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 4 years ago by
 Status changed from needs_review to needs_work
Doc test failures:
sage t projective_morphism.py # 6 doctests failed
comment:9 Changed 4 years ago by
 Commit changed from 684b502617d603bfdc5e3416aaeaed05499f6f3f to 07592c745b9d0c24fd137d95d949ba8130d0a0f0
Branch pushed to git repo; I updated commit sha1. New commits:
07592c7  Fixed a logic error

comment:10 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:11 Changed 4 years ago by
 Status changed from needs_review to needs_work
The code snippents above are resolved now, but here are a couple more issues:
 the docs are still wrong see comment 4 above
 for subschemes the points being returned are in the ambient space and not the subscheme
 the following two examples fail
P.<x,y>=ProjectiveSpace(CC,1) H=End(P) f=H([x^22*y^2,y^2]) f.rational_preimages(P([0,1]))
The previous example also fails for RR
z = var('z') K.<w>=NumberField(z^3+2) R.<z>=PolynomialRing(K) L.<v>=K.extension(z^2+z+1) P.<x,y>=ProjectiveSpace(L,1) H=End(P) f=H([x^32*y^3,v*y^3]) f.rational_preimages(P([0,1]))
comment:12 Changed 4 years ago by
 Commit changed from 07592c745b9d0c24fd137d95d949ba8130d0a0f0 to 4c94954f4a055c3a88cb22fe90cb9f05ecc36c33
Branch pushed to git repo; I updated commit sha1. New commits:
4c94954  Fixed documentation, added logic for precision fields

comment:13 Changed 4 years ago by
 Commit changed from 4c94954f4a055c3a88cb22fe90cb9f05ecc36c33 to c6bb45162881ef1c2c8472d00e7b0a15e76a5b1d
comment:14 Changed 4 years ago by
 Commit changed from c6bb45162881ef1c2c8472d00e7b0a15e76a5b1d to 928941e6a3df966c89ae97c7bf0250bd049e264c
Branch pushed to git repo; I updated commit sha1. New commits:
928941e  Coerced points into proper domain

comment:15 Changed 4 years ago by
 Status changed from needs_work to needs_review
Fixed all of the problems found above, ran doc test, all docs passed on my end, setting to review
comment:16 Changed 4 years ago by
 Status changed from needs_review to needs_work
Actually, this example is not fixed
P.<x,y>=ProjectiveSpace(CC,1) H=End(P) f=H([x^22*y^2,y^2]) f.rational_preimages(P([0,1]))
but the other one is.
Also some minor things:
 Both docstrings still says requires QQ and endomorphism of projective space. Neither are still true.
 There are two lines with terminating white space 2519, 2550.
 Coordinates is spelled wrong on 2604.
comment:17 Changed 4 years ago by
opps, my mistake, that example is fixed I just didn't get the latest commits on my last pull. There are still three minor issues:
 line 2552 trailing white space
 doc string references endomorphism of projective space, but since you're allowing subschemes, you just need 'an endomorphism'.
 coordinates is spelled wrong line 2611
comment:18 Changed 4 years ago by
 Commit changed from 928941e6a3df966c89ae97c7bf0250bd049e264c to 48eebc0292ffbb4cb4c36f2d9fb159d3bc41f653
Branch pushed to git repo; I updated commit sha1. New commits:
48eebc0  Fixed minor typos

comment:19 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:20 Changed 4 years ago by
 Status changed from needs_review to positive_review
this looks good now.
comment:21 Changed 4 years ago by
 Branch changed from u/jdefaria/ticket/16986 to 48eebc0292ffbb4cb4c36f2d9fb159d3bc41f653
 Resolution set to fixed
 Status changed from positive_review to closed
comment:22 Changed 4 years ago by
 Commit 48eebc0292ffbb4cb4c36f2d9fb159d3bc41f653 deleted
New commits:
Added functionality for Number Fields