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

minimal periodic points code improvement #20059

Closed
bhutz opened this issue Feb 15, 2016 · 10 comments
Closed

minimal periodic points code improvement #20059

bhutz opened this issue Feb 15, 2016 · 10 comments

Comments

@bhutz
Copy link

bhutz commented Feb 15, 2016

In projective morphism the code block for minimal periodic points around line 3003

        if not minimal:
            return points
        else:
            rem_indices = []
            for i in range(len(points)-1,-1,-1):
                # iterate points to check if minimal
                P = points[i]
                for j in range(1,n):
                    P = self(P)
                    if P == points[i]:
                        points.pop(i)
                        break
            return points

has an unused list and could use some additional comments to explain what this is doing.

CC: @sagetrac-gjorgenson

Component: algebraic geometry

Author: Ben Hutz

Branch: 3c9adc3

Reviewer: Rebecca Lauren Miller

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

@bhutz bhutz added this to the sage-7.1 milestone Feb 15, 2016
@bhutz
Copy link
Author

bhutz commented Mar 5, 2016

Branch: u/bhutz/ticket/20059

@bhutz
Copy link
Author

bhutz commented Mar 5, 2016

Commit: 3c9adc3

@bhutz
Copy link
Author

bhutz commented Mar 5, 2016

comment:2

In addition to adding the documentation to the minimal section I made three improvements to the function

  • allowed the user to pass in a ring where the points should be defined

  • extended the functionality to include finite fields

  • allowed the user to choose the method used


New commits:

3c9adc320059: improvement to projective periodic_points function

@bhutz
Copy link
Author

bhutz commented Mar 5, 2016

Author: Ben Hutz

@sagetrac-rlmiller
Copy link
Mannequin

sagetrac-rlmiller mannequin commented Mar 8, 2016

Reviewer: Rebecca Miller

@sagetrac-rlmiller
Copy link
Mannequin

sagetrac-rlmiller mannequin commented Mar 9, 2016

comment:4

Looks good to me.

@vbraun
Copy link
Member

vbraun commented Mar 9, 2016

Changed branch from u/bhutz/ticket/20059 to 3c9adc3

@jdemeyer
Copy link

Changed commit from 3c9adc3 to none

@jdemeyer
Copy link

comment:6

What do you prefer: "Rebecca Lauren Miller" or "Rebecca Miller"?

@sagetrac-rlmiller
Copy link
Mannequin

sagetrac-rlmiller mannequin commented Mar 22, 2016

Changed reviewer from Rebecca Miller to Rebecca Lauren Miller

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