Opened 8 months ago

Closed 7 months ago

#22265 closed defect (fixed)

Fix Dynatomic Polynomials

Reported by: rlmiller Owned by:
Priority: minor Milestone: sage-7.6
Component: algebra Keywords:
Cc: Ben, Hutz Merged in:
Authors: Rebecca Lauren Miller Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 994717c (Commits) Commit: 994717c9c79f27c32abca28cd00d07b423934da7
Dependencies: Stopgaps:

Description

Need to add NotImplementedError? to try/except in homogenize in affine_morphism. Need to fix dynatomic error that returns wrong parent in projective_morphsim. Current examples of failure:

  R.<c>=QQ[]
A.<z>=AffineSpace(R,1)
H=End(A)
f=H([z^2+c])
f.dynatomic_polynomial([1,2])
R.<c>=QQ[]
P.<x,y>=ProjectiveSpace(R,1)
H=End(P)
f=H([x^2+c*y^2,y^2])
f.dynatomic_polynomial([1,2]).parent()
R.<c>=QQ[]
P.<x,y>=ProjectiveSpace(ZZ,1)
H=End(P)
f=H([x^2+y^2,(1)*y^2 + (1)*x*y])
f.dynatomic_polynomial([1,2]).parent()

Change History (16)

comment:1 Changed 8 months ago by bhutz

Another failure as of 7.6.beta2

sage: P.<x,y> = ProjectiveSpace(CC, 1)
sage: H = Hom(P,P)
sage: f = H([x^2-CC.0/3*y^2, y^2])
sage: f.dynatomic_polynomial(2)

this is line 853 in the projective_morphism.py file. The expected value is also wrong.

comment:2 Changed 8 months ago by bhutz

What is happening in that example is that the quo_rem fails to return a nonzero reminder, so then maxima is tried. The division in maxima (line 964) is not checked to see if the remainder is zero (which it is not).

This example is unable to be exactly divided, so it should return the rational function.

comment:3 Changed 8 months ago by rlmiller

  • Branch set to u/rlmiller/master

comment:4 Changed 8 months ago by rlmiller

  • Branch changed from u/rlmiller/master to u/rlmiller/22265
  • Commit set to 940a51be5fa4e9dc54b558bd19de41ff9a845c90
  • Status changed from new to needs_review

New commits:

940a51b22265 fixed errors in dynatomic polynomial

comment:5 Changed 7 months ago by bhutz

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

Both examples where the base ring is a polynomial ring still do not work.

The example you added with

f.dynatomic_polynomial([0,1]).parent()

is still the m=0 case, which is not what you changed. I would choose a different period for the example.

You need to add some readability spacing such as around = and ==

The complex example is fixed. I think it would be good to also add an example where the division succeeds, CC.0/2 or some other finite decimal expansion.

comment:6 Changed 7 months ago by git

  • Commit changed from 940a51be5fa4e9dc54b558bd19de41ff9a845c90 to 288c766caf640e52d1e5650927f02bbc5929950a

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

288c76622265 error fixes for dynatomic polynomials

comment:7 Changed 7 months ago by rlmiller

  • Status changed from needs_work to needs_review

comment:8 Changed 7 months ago by bhutz

  • Status changed from needs_review to needs_work

The original failures listed in the ticket now work. However, I found a number of additional examples that fail. Mostly it is for the affine computation, but apparently there is also no input checking done on the n parameter in both affine and projective.

Needs input checking:

P.<x,y> = ProjectiveSpace(ZZ, 1)
H = Hom(P,P)
f = H([x^2-5*y^2, y^2])
D=f.dynatomic_polynomial(-3)
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial(-3)
D.parent(),D
P.<x,y> = ProjectiveSpace(ZZ, 1)
H = Hom(P,P)
f = H([x^2-5*y^2, y^2])
D=f.dynatomic_polynomial([1,0])
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial([1,0])
D.parent(),D

Other failures:

R.<x>=QQ[]
K.<i>=R.quo(x^2+1)
P.<x,y> = ProjectiveSpace(K, 1)
H = Hom(P,P)
f = H([x^2-i/3*y^2, y^2])
D=f.dynatomic_polynomial([2,1])
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial([2,1])
D.parent(),D
Traceback (click to the left of this block for traceback)
...
TypeError: lift() takes exactly 2 arguments (1 given)
P.<x,y> = ProjectiveSpace(Zp(3), 1)
H = Hom(P,P)
f = H([x^2-5*y^2, y^2])
D=f.dynatomic_polynomial([1,2])
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial([1,2])
D.parent(),D
Traceback (click to the left of this block for traceback)
...
TypeError: not a constant polynomial
R.<c,d>=QQbar[]
P.<x,y>=ProjectiveSpace(R,1)
H=End(P)
f=H([x^2+c*y^2,d*y^2])
D=f.dynatomic_polynomial([1,2])
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial([1,2])
D.parent(),D
Traceback (click to the left of this block for traceback)
...
TypeError: no conversion of this ring to a Singular ring defined
R.<c,d>=QQbar[]
P.<x,y>=ProjectiveSpace(QQbar,1)
H=End(P)
f=H([x^2+QQbar(sqrt(3))*y^2,2*y^2])
D=f.dynatomic_polynomial([1,2])
print D.parent(),D
F=f.dehomogenize(1)
D=F.dynatomic_polynomial([1,2])
D.parent(),D
Traceback (click to the left of this block for traceback)
...
TypeError: not a constant polynomial

comment:9 Changed 7 months ago by git

  • Commit changed from 288c766caf640e52d1e5650927f02bbc5929950a to e270735e739017a412fbd93408d2f4d3e6aa5be8

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

e27073522265 Fixed Dynatomic Polynomial failures and added examples

comment:10 Changed 7 months ago by rlmiller

  • Status changed from needs_work to needs_review

comment:11 Changed 7 months ago by bhutz

  • Status changed from needs_review to needs_work

Looks like these are all fixed except the input checking. The following should also be returning 0

P.<x,y> = ProjectiveSpace(ZZ, 1)
H = Hom(P,P)
f = H([x^2-5*y^2, y^2])
f.dynatomic_polynomial([3,0])

comment:12 Changed 7 months ago by git

  • Commit changed from e270735e739017a412fbd93408d2f4d3e6aa5be8 to d10f4e067329939cba9bfc156c6e6410c34b74b1

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

d10f4e022265 Fixed error when n=0

comment:13 Changed 7 months ago by rlmiller

  • Status changed from needs_work to needs_review

comment:14 Changed 7 months ago by git

  • Commit changed from d10f4e067329939cba9bfc156c6e6410c34b74b1 to 994717c9c79f27c32abca28cd00d07b423934da7

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

994717c22265 Code Beautification

comment:15 Changed 7 months ago by bhutz

  • Status changed from needs_review to positive_review

comment:16 Changed 7 months ago by vbraun

  • Branch changed from u/rlmiller/22265 to 994717c9c79f27c32abca28cd00d07b423934da7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.