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

Add iterator method in QQ for Calkin-Wilf sequence #28282

Open
bgillesp opened this issue Jul 30, 2019 · 13 comments
Open

Add iterator method in QQ for Calkin-Wilf sequence #28282

bgillesp opened this issue Jul 30, 2019 · 13 comments

Comments

@bgillesp
Copy link
Contributor

Add a method to the set of rational numbers to produce an iterator for the Calkin-Wilf sequence, an explicit ordering of the positive rationals with some interesting features. Background:

CC: @slel

Component: combinatorics

Keywords: sequence, iterator

Author: Bryan Gillespie

Branch/Commit: u/bgillespie/add_iterator_method_in_qq_for_calkin_wilf_sequence @ 9b82dac

Reviewer: Samuel Lelièvre

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

@bgillesp bgillesp added this to the sage-8.9 milestone Jul 30, 2019
@bgillesp
Copy link
Contributor Author

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2019

Commit: 9b82dac

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2019

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

9b82dacAdd literature reference and improve performance

@bgillesp
Copy link
Contributor Author

Author: Bryan Gillespie

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:4

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@mkoeppe
Copy link
Member

mkoeppe commented Apr 14, 2020

comment:5

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@slel
Copy link
Member

slel commented Sep 1, 2020

comment:6

That would be a nice addition!

The branch needs rebasing. In addition I suggest considering these minor changes:

-        p, q = ZZ(1), ZZ(1)
+        p = q = ZZ.one()
         if all_rationals:
-            yield self(0)
+            yield self.zero()
             while True:
-                yield self(p / q)
-                yield -self(p / q)
+                yield self((p, q))
+                yield self((-p, q))
                 p, q = q, 2 * (p // q) * q - p + q
         else:
             while True:
-                yield self(p / q)
+                yield self((p, q))
                 p, q = q, 2 * (p // q) * q - p + q

@slel
Copy link
Member

slel commented Sep 1, 2020

Reviewer: Samuel Lelièvre

@nbruin
Copy link
Contributor

nbruin commented Sep 1, 2020

comment:7

We've been there already:

sagemath/sagetrac-mirror@45dc1b9/src/sage/rings/rational_field.py

It's indeed a cute sequence. The generating formula is surprisingly simple, but most problems that require iterating over the rationals are better served by another order (indeed, it's since been replaced by iteration by height):

https://github.com/sagemath/sagetrac-mirror/commits/55ad3590cb7d993f38c9104ea87da24b21567009

@slel
Copy link
Member

slel commented Sep 1, 2020

comment:8

Thanks @nbruin for pointing out the initial iterator for QQ
used this sequence, and for the pointers to the commits
implementing it and then replacing it with a better default.

This ticket aims to provide a Calkin-Wilf iterator as distinct
from the standard iterator for QQ.

Would you advise against it? Would this Calkin-Wilf iterator
better be given as an example in the tutorial on iterators?
Maybe with a reference from the documentation of QQ?

@nbruin
Copy link
Contributor

nbruin commented Sep 2, 2020

comment:9

Replying to @slel:

Thanks @nbruin for pointing out the initial iterator for QQ
used this sequence, and for the pointers to the commits
implementing it and then replacing it with a better default.

This ticket aims to provide a Calkin-Wilf iterator as distinct
from the standard iterator for QQ.

Would you advise against it? Would this Calkin-Wilf iterator
better be given as an example in the tutorial on iterators?
Maybe with a reference from the documentation of QQ?

I'd think so. The formula is exceedingly simple: see the original implementation that was essentially a one-liner.

I was originally enthusiastic when I learned about the formula, but I don't think the sequence really has wider applications. Including it would be more of encyclopedic interest, and I think QQ is not the right place for that. We already have OEIS. Interfacing with that makes more sense. There is in fact a (lousy) sage program listed there. It would make a lot more sense if there were an iterator there.

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Member

mkoeppe commented Feb 13, 2021

comment:11

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe
Copy link
Member

mkoeppe commented Jul 19, 2021

comment:12

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

5 participants