Opened 2 years ago
Closed 2 years ago
#27779 closed enhancement (fixed)
Py3: Fix dynamics.arithmetic_dynamics for python3
Reported by:  vklein  Owned by:  

Priority:  major  Milestone:  sage8.8 
Component:  python3  Keywords:  
Cc:  bhutz  Merged in:  
Authors:  Vincent Klein  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  e26a443 (Commits, GitHub, GitLab)  Commit:  e26a443fba787f4a66c0c1b76b84558b61987489 
Dependencies:  Stopgaps: 
Description
Fix dynamics.arithmetic_dynamics for python3. One failling doctest remains.
Change History (16)
comment:1 Changed 2 years ago by
 Branch set to u/vklein/py3__fix_dynamics_arithmetic_dynamics_for_python3
comment:2 Changed 2 years ago by
 Commit set to ccf846c2be2b5015a2595370475433c4c5aeb1c7
 Status changed from new to needs_review
comment:3 Changed 2 years ago by
some of the new lines must be tagged # long time
too, as they depend on #long results
+ sage: lst, label = f.automorphism_group(return_functions=True, iso_type=True) # long time + sage: sorted(lst, key=str), label
comment:4 Changed 2 years ago by
 Commit changed from ccf846c2be2b5015a2595370475433c4c5aeb1c7 to b04931f9f85f85e97701acc3184cc11da459d3d3
Branch pushed to git repo; I updated commit sha1. New commits:
b04931f  Trac #27779: Add missing `# long time` flag

comment:5 Changed 2 years ago by
Thanks ! Missing tag added.
comment:6 Changed 2 years ago by
maybe add # abs tol 1e11
to the last failing doctest ?
comment:7 Changed 2 years ago by
It works. You mean values (x^4  2.86348722511320e12*y^4 : 1.41421356237310*y^4)
and (x^4 + 7.74491581978509e13*y^4 : 1.41421356237310*y^4)
are both valid results for this doctest ?
comment:8 Changed 2 years ago by
I thought so, but maybe this is not so clear. We should cc the authors of that piece of code.
comment:9 Changed 2 years ago by
Git blame says it's from Ben Hutz but he is not listed in sage account list.
comment:10 Changed 2 years ago by
 Commit changed from b04931f9f85f85e97701acc3184cc11da459d3d3 to d2ad26030d76568687134cc5536998a28c5a3b4a
Branch pushed to git repo; I updated commit sha1. New commits:
d2ad260  Trac #27779: Add `# abs tol 1e11` tag to doctest

comment:11 Changed 2 years ago by
 Cc bhutz added
comment:12 Changed 2 years ago by
Perhaps a cleaner fix here would be to change the doctest to
g,m = f.reduced_form(smallest_coeffs=False); m
The matrix is it producing for the conjugation is always an integer matrix, but just working over RR to conjugate and then conjugate by the inverse is introducing numerical error, so the resulting function is not an exact match of the original. So, the error that is cropping up in the doctest actually has nothing to do with the functionality of .reduced_form(), rather it is just a consequence of conjugating over a numerical field.
You should probably do the same thing for the doctest immediately following as I would have expected the same kind of issue over CC.
comment:13 Changed 2 years ago by
 Commit changed from d2ad26030d76568687134cc5536998a28c5a3b4a to e26a443fba787f4a66c0c1b76b84558b61987489
Branch pushed to git repo; I updated commit sha1. New commits:
e26a443  Trac #27779: Test the matrix result only for ...

comment:14 Changed 2 years ago by
Ok it's done.
Thank you for your help Ben.
comment:15 Changed 2 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
Ça me parait correct.
comment:16 Changed 2 years ago by
 Branch changed from u/vklein/py3__fix_dynamics_arithmetic_dynamics_for_python3 to e26a443fba787f4a66c0c1b76b84558b61987489
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Trac 27779: Fix arithmetic_dynamics for py3