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

enhance ratpoints interface #15554

Closed
JohnCremona opened this issue Dec 20, 2013 · 28 comments
Closed

enhance ratpoints interface #15554

JohnCremona opened this issue Dec 20, 2013 · 28 comments

Comments

@JohnCremona
Copy link
Member

In sage/libs/ratpoints there is a cython interface to the standard ratpoints package (Michael Stoll's efficient C library for point searching on curves of the form y^2=f(x) for polynomials f(x) of any degree).

The interface currently only allows the user to specify the coefficients of the polynomial, a height bound, and a maximum number of points to be found. But ratpoints has other parameters, in particular upper and lower bounds for the denominator of x (useful in searching for integral points) and the ability to search only for x in one or more real intervals.

We propose to enhance the interface to allow non-defaults for all the ratpoints parameters, while remaining completely backwards compatible.

This description was edited using sage -dev edit-ticket --ticket 15554 .

Component: number theory

Author: John Cremona

Branch/Commit: public/interfaces/ratpoints-15554 @ 1d5ec7f

Reviewer: Travis Scrimshaw

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

@JohnCremona

This comment has been minimized.

@JohnCremona
Copy link
Member Author

Branch: u/cremona/ticket/15554

@vbraun
Copy link
Member

vbraun commented Dec 20, 2013

comment:4

The commit should be filled in automatically...

@vbraun
Copy link
Member

vbraun commented Dec 20, 2013

Commit: 3472a85

@JohnCremona
Copy link
Member Author

Author: John Cremona

@JohnCremona
Copy link
Member Author

comment:5

Replying to @vbraun:

The commit should be filled in automatically...

Thanks!

@JohnCremona
Copy link
Member Author

comment:6

I pushed branch u/cremona/ticket/15554 to trac.

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

comment:7

I'm going to try removing the branch and then reset it.

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

Changed branch from u/cremona/ticket/15554 to none

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

Changed commit from 3472a85 to none

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

Commit: 3472a85

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

Branch: u/cremona/ticket/15554

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

comment:9

Hmmm...Perhaps try rebasing it off the develop branch and see if that works?

@tscrim
Copy link
Collaborator

tscrim commented Dec 21, 2013

comment:10

From https://github.com/sagemath/sagetrac-mirror/commits/u/cremona/ticket/15554 it seems like this is the master branch, so there's no differences in the commits (and possibly is why there's no (commit) tag).

@vbraun
Copy link
Member

vbraun commented Dec 21, 2013

comment:11

Correct, 3472a85 is Sage-6.0 master.

@JohnCremona
Copy link
Member Author

comment:12

Replying to @vbraun:

Correct, 3472a85 is Sage-6.0 master.

Volker, you set that commit field manually! The hash of the commit I actually pushed is
8527d9f

If you cannot see that it must not have pushed properly, I am pushing it again now using git directly and not sage -dev. But it failed:

jec@atkin:~/sage (branch: trac15554-ratpoints)$ git push trac u/cremona/ticket/15554
X11 forwarding request failed on channel 0
error: src refspec u/cremona/ticket/15554 does not match any.
error: failed to push some refs to 'ssh://git@trac.sagemath.org:2222/sage.git'

@vbraun
Copy link
Member

vbraun commented Dec 21, 2013

comment:13

Try

git push trac HEAD:u/cremona/ticket/15554

@vbraun
Copy link
Member

vbraun commented Dec 21, 2013

comment:14

PS: I didn't set the commit field myself, that was a trac plugin (though its a bit confusing that it shows up under my name)

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

comment:15

You might also want to try:

git push --set-upstream trac u/cremona/ticket/15554

then you can just use git push on every subsequent time.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 22, 2013

Changed commit from 3472a85 to 8527d9f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 22, 2013

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

8527d9ftrac 15554: enhance ratpoints interface

@JohnCremona
Copy link
Member Author

comment:17

Replying to @tscrim:

You might also want to try:

git push --set-upstream trac u/cremona/ticket/15554

then you can just use git push on every subsequent time.

Thanks, I had forgotten that syntax. I think it has now pushed ok, but as usual the push command takes a very long time and has not given me back a prompt yet.

Thanks to both for the help.

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

comment:18

Replying to @JohnCremona:

I think it has now pushed ok, but as usual the push command takes a very long time and has not given me back a prompt yet.

This is "known" (at least to Volker and me -- I forget which sage-devel/sage-git topic Volker mentioned having this problem), and if it hangs, you can just ctrl-C. By that point it usually has already pushed.

I will make some review changes in a minute.

Best,

Travis

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

Changed branch from u/cremona/ticket/15554 to public/interfaces/ratpoints-15554

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

Changed commit from 8527d9f to 1d5ec7f

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

comment:19

Okay, I've made a (trivial) review change, I broke up the one lone line. If you're happy with my change, then this is a positive review.


New commits:

1d5ec7fMinor review tweak.
5170720Merge branch 'u/cremona/ticket/15554' of trac.sagemath.org:sage into public/interfaces/ratpoints-15554

@tscrim
Copy link
Collaborator

tscrim commented Dec 22, 2013

Reviewer: Travis Scrimshaw

@JohnCremona
Copy link
Member Author

comment:20

I am happy. I pulled your additional commit and was able to view the diff using

git diff HEAD^ HEAD

and it is as you say. Thanks for the review, and feel free to give this a positive review!

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