Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix is_preperiodic function domain check #23814

Closed
pfili opened this issue Sep 9, 2017 · 13 comments
Closed

Fix is_preperiodic function domain check #23814

pfili opened this issue Sep 9, 2017 · 13 comments

Comments

@pfili
Copy link

pfili commented Sep 9, 2017

Currently, if we define a DynamicalSystem_projective without specifying the underlying domain (which is probably the most likely way the user will do this), the init function recreates the underlying projective space. As a result, any function that checks if the domain of a point "is" the domain of the map will fail, as is_preperiodic currently does:

sage: R.<X> = PolynomialRing(QQ)
sage: K.<a> = NumberField(X^2 + X - 1)
sage: P.<x,y> = ProjectiveSpace(K,1)
sage: f = DynamicalSystem_projective([x^2-2*y^2, y^2])
sage: Q = P.point([a,1])
sage: f.nth_iterate(Q,2) == Q
True
sage: Q.is_preperiodic(f)
Traceback (most recent call last):
...
TypeError: point must be in domain of map
sage: f.domain() is P
False
sage: f.domain() == P # It's basically the same but the space is not uniquely represented in memory
True

Instead, is_preperiodic should be using == to check that the domains are equivalent, rather than exactly the same under "is", and then things will work as expected. This fix changes that.

Depends on #23497

CC: @bhutz

Component: dynamics

Author: Paul Fili

Branch/Commit: 2a68ffb

Reviewer: Ben Hutz

Issue created by migration from https://trac.sagemath.org/ticket/23814

@pfili pfili added this to the sage-8.1 milestone Sep 9, 2017
@pfili
Copy link
Author

pfili commented Sep 9, 2017

Dependencies: #23497

@pfili
Copy link
Author

pfili commented Sep 9, 2017

Branch: u/paulfili/preperiodic-fix

@pfili
Copy link
Author

pfili commented Sep 9, 2017

Last 10 new commits:

79378c3Merge branch 8.1.beta3 into t/23497/arith_dyn
80ff2e623497: some fixes from review
fdf726523497: doc formatting INPUT/OUTPUT
cc5010823497: use __classcall_private__
4bbdcdeFixing things with __classcall_private__.
7408d79Cleaning up the doc and improving the code.
d00cea223497: a couple minor fixes
300a3b1Merge branch 8.1.beta4 into t/23497/arith_dyn-23497
bdbf658Fixing typo in doctest.
9c3e7cfFix domain check for is_preperiodic from "is" to "=="

@pfili
Copy link
Author

pfili commented Sep 9, 2017

Commit: 9c3e7cf

@bhutz
Copy link

bhutz commented Sep 9, 2017

Changed branch from u/paulfili/preperiodic-fix to u/bhutz/preperiodic-fix

@bhutz
Copy link

bhutz commented Sep 9, 2017

Reviewer: Ben Hutz

@bhutz
Copy link

bhutz commented Sep 9, 2017

comment:5

Looks fine. Just fixed a minor typo.


New commits:

1f1a81523814: minor typo fix

@bhutz
Copy link

bhutz commented Sep 9, 2017

Changed commit from 9c3e7cf to 1f1a815

@bhutz
Copy link

bhutz commented Sep 11, 2017

comment:6

merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2017

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

2eecc68Merge branch 8.1.beta5 into t/23497/arith_dyn-23497
2a68ffbMerge branch 't/23497/arith_dyn-23497' into t/23814/preperiodic-fix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2017

Changed commit from 1f1a815 to 2a68ffb

@bhutz
Copy link

bhutz commented Sep 11, 2017

comment:8

fixed

@vbraun
Copy link
Member

vbraun commented Sep 24, 2017

Changed branch from u/bhutz/preperiodic-fix to 2a68ffb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants