Opened 7 months ago
Closed 7 months ago
#27779 closed enhancement (fixed)
Py3: Fix dynamics.arithmetic_dynamics for python3
Reported by: | vklein | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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) | Commit: | e26a443fba787f4a66c0c1b76b84558b61987489 |
Dependencies: | Stopgaps: |
Description
Fix dynamics.arithmetic_dynamics for python3. One failling doctest remains.
Change History (16)
comment:1 Changed 7 months ago by
- Branch set to u/vklein/py3__fix_dynamics_arithmetic_dynamics_for_python3
comment:2 Changed 7 months ago by
- Commit set to ccf846c2be2b5015a2595370475433c4c5aeb1c7
- Status changed from new to needs_review
comment:3 Changed 7 months 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 7 months 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 7 months ago by
Thanks ! Missing tag added.
comment:6 Changed 7 months ago by
maybe add # abs tol 1e-11
to the last failing doctest ?
comment:7 Changed 7 months ago by
It works. You mean values (x^4 - 2.86348722511320e-12*y^4 : 1.41421356237310*y^4)
and (x^4 + 7.74491581978509e-13*y^4 : 1.41421356237310*y^4)
are both valid results for this doctest ?
comment:8 Changed 7 months 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 7 months ago by
Git blame says it's from Ben Hutz but he is not listed in sage account list.
comment:10 Changed 7 months 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 1e-11` tag to doctest
|
comment:11 Changed 7 months ago by
- Cc bhutz added
comment:12 Changed 7 months 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 7 months 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 7 months ago by
Ok it's done.
Thank you for your help Ben.
comment:15 Changed 7 months ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
Ça me parait correct.
comment:16 Changed 7 months 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