Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16961 closed defect (fixed)

Fix Dynatomic Polynomials to work over the Complex Numbers

Reported by: jdefaria Owned by:
Priority: major Milestone: sage-6.4
Component: algebraic geometry Keywords:
Cc: bhutz Merged in:
Authors: Joao Alberto de Faria Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 8865d23 (Commits) Commit:
Dependencies: Stopgaps:

Description

P.<x,y> = ProjectiveSpace(CC, 1)
H = Hom(P,P)
f = H([x^2+(1+CC.0)*y^2,y^2])
f.dynatomic_polynomial(2)

returns the wrong answer

Change History (15)

comment:1 Changed 3 years ago by jdefaria

  • Branch set to u/jdefaria/ticket/16961
  • Created changed from 09/10/14 17:43:00 to 09/10/14 17:43:00
  • Modified changed from 09/10/14 17:43:00 to 09/10/14 17:43:00

comment:2 Changed 3 years ago by bhutz

  • Commit set to d4a35de7e0f5cd03a9e78690ef5b462d1fc63864

Two things for now. This does not pass its doc tests, so run them and fix them.

2nd, you went overboard on the spaces. If you want to look at the actual conventions see: http://www.sagemath.org/doc/developer/coding_basics.html#docstring-markup-with-rest-and-sphinx


New commits:

d4a35deFixed Dynatomic Poly to work over CC

comment:3 Changed 3 years ago by git

  • Commit changed from d4a35de7e0f5cd03a9e78690ef5b462d1fc63864 to 353e842e0041e766b634b032abb8a551782125a0

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

353e842Fixed spacing typos and fixed doctest errors

comment:4 Changed 3 years ago by jdefaria

  • Status changed from new to needs_review

comment:5 Changed 3 years ago by bhutz

  • Status changed from needs_review to needs_work

The 2nd todo block is not needed (the one in the examples) because that is already said in the main todo block.

Also, the affine ones are still not working for complex values, although the issue appears to stem from homogenize and should be fixed with trac #16838. Is this ticket merged with the latest develop so that you have the fixes from 16838?

A.<x>=AffineSpace(CC,1)
H=Hom(A,A)
f=H([x^2+CC.0])
f.dynatomic_polynomial(2)

btw, the affine examples are lacking ' = ' in the examples, so you could address that at the same time.

comment:6 Changed 3 years ago by git

  • Commit changed from 353e842e0041e766b634b032abb8a551782125a0 to 92417c95137719e4f74cf8f75c7b43d757346a11

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

089ff63Merge branch 'master' into ticket/16961
92417c9Merged with most recent beta, added another example

comment:7 Changed 3 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:8 Changed 3 years ago by bhutz

  • Status changed from needs_review to needs_work

I hate to say it, but there is something wrong with the spacing of the dynatomic polynomial examples in affine_morphism. They are not type setting correctly. Probably the '::' need to be back farther.

Otherwise the functionality checks out. The merge with the latest beta fixed the affine issues.

comment:9 Changed 3 years ago by git

  • Commit changed from 92417c95137719e4f74cf8f75c7b43d757346a11 to 8865d236ffb0bf3900d822af4f27208696323304

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

8865d23Fixed formatting for examples for the Affine Case

comment:10 Changed 3 years ago by jdefaria

  • Status changed from needs_work to needs_review

comment:11 Changed 3 years ago by bhutz

  • Status changed from needs_review to positive_review

This looks good now.

comment:12 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

reviewer name

comment:13 Changed 3 years ago by bhutz

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

comment:14 Changed 3 years ago by vbraun

  • Branch changed from u/jdefaria/ticket/16961 to 8865d236ffb0bf3900d822af4f27208696323304
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:15 Changed 3 years ago by kcrisman

  • Authors changed from Joao de Faria to Joao Alberto de Faria
  • Commit 8865d236ffb0bf3900d822af4f27208696323304 deleted
Note: See TracTickets for help on using tickets.