-
-
Notifications
You must be signed in to change notification settings - Fork 688
Subsets of primes defined by congruence conditions #41122
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
base: develop
Are you sure you want to change the base?
Conversation
|
Documentation preview for this PR (built with commit 1ef8180; changes) is ready! 🎉 |
Co-authored-by: Martin Rubey <[email protected]>
Co-authored-by: Martin Rubey <[email protected]>
Co-authored-by: Martin Rubey <[email protected]>
|
Question: do we want methods for accessing |
Ok, I did it :-) |
| else: | ||
| return P == other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| else: | |
| return P == other | |
| return P == other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that it is equivalent but I prefer keeping the else here because I find it more logic.
Co-authored-by: Martin Rubey <[email protected]>
Co-authored-by: Martin Rubey <[email protected]>
Co-authored-by: Martin Rubey <[email protected]>
|
I have one more comment: I am quite sure it would be better to get rid of Thus, the most naive version would be as below. I am sure one can do better - in the worst case make a special case for index 6eb5d2a52f6..e4c1480cd28 100644
--- a/src/sage/sets/primes.py
+++ b/src/sage/sets/primes.py
@@ -877,17 +877,19 @@ class Primes(Set_generic, UniqueRepresentation):
"""
if other is ZZ:
return self
- if not isinstance(other, Primes):
- raise NotImplementedError("boolean operations are only implemented with other sets of prime numbers")
- modulus = self._modulus.lcm(other._modulus)
- classes = [c for c in range(modulus)
- if (c % self._modulus in self._classes
- and c % other._modulus in other._classes)]
- exceptions = {x: b for x, b in self._exceptions.items()
- if not b or x in other}
- exceptions.update((x, b) for x, b in other._exceptions.items()
- if not b or x in self)
- return Primes(modulus, classes, exceptions)
+ if isinstance(other, Primes):
+ modulus = self._modulus.lcm(other._modulus)
+ classes = [c for c in range(modulus)
+ if (c % self._modulus in self._classes
+ and c % other._modulus in other._classes)]
+ exceptions = {x: b for x, b in self._exceptions.items()
+ if not b or x in other}
+ exceptions.update((x, b) for x, b in other._exceptions.items()
+ if not b or x in self)
+ return Primes(modulus, classes, exceptions)
+
+ # assume that other is a finite iterable
+ return Primes(modulus=0, classes=[x for x in other if x in self]) |
|
Ok, I will do it. |
src/sage/sets/primes.py
Outdated
| elif self.is_finite(): | ||
| modulus = 1 | ||
| classes = [] | ||
| exceptions = {x: True for x in self if x in other} | ||
| else: | ||
| # we try to enumerate the elements of "other" | ||
| modulus = 1 | ||
| classes = [] | ||
| exceptions = {x: True for x in list(other) if x in self} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you betting on self being smaller than other if self.is_finite? Personally, I wouldn't, but there are problems in either case:
x in otherconsumes the iterator, so the following might be surprising:
sage: P = Primes(modulus=0, classes=range(30))
sage: P.intersection(range(10))
Finite set of prime numbers: 2, 3, 5, 7
sage: P.intersection(reversed(range(10)))
Finite set of prime numbers: 2
-
list(other)wastes of time and memory -
It may make sense to do a special case for range (untested - I guess one could even accomodate
step). This will only make a difference ifother.stopis large, I don't know whether this occurs in practice.
| elif self.is_finite(): | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in self if x in other} | |
| else: | |
| # we try to enumerate the elements of "other" | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in list(other) if x in self} | |
| elif isinstance(other, range) and other.step == 1: | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {} | |
| x = other.start - 1 | |
| while True: | |
| x = self.next(x) | |
| if x >= other.stop: | |
| break | |
| exceptions[x] = True | |
| else: | |
| # we try to enumerate the elements of "other" | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in other if x in self} |
The simpler suggestion is:
| elif self.is_finite(): | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in self if x in other} | |
| else: | |
| # we try to enumerate the elements of "other" | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in list(other) if x in self} | |
| else: | |
| # we try to enumerate the elements of "other" | |
| modulus = 1 | |
| classes = [] | |
| exceptions = {x: True for x in other if x in self} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you betting on
selfbeing smaller thanotherifself.is_finite?
No, not necessarily.
But if self is finite and other is infinite, we can compute the intersection; with your suggestions, it will loop forever, no?
* `x in other` consumes the iterator, so the following might be surprising:
It is one reason why I used list(other); the other one was that I get an error message if other is infinite.
I agree nonetheless that this is less efficient and uses more memory.
* It may make sense to do a special case for range (untested - I guess one could even accomodate `step`). This will only make a difference if `other.stop` is large, I don't know whether this occurs in practice.
If other.stop is large and self is finite. Which is already covered by my proposal since I first check if self is finite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If other is an infinite iterator and x does not appear in other then x in other will also loop forever.
Of course, there are interesting special cases of iterables, most importantly those that implement __contains__.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, taking into account the above, here is my suggestion for intersection. I think the special case for range objects should generalize to arbitrary step, but I don't have enough patience right now.
index 8f81bc11995..a72ff85f1d0 100644
--- a/src/sage/sets/primes.py
+++ b/src/sage/sets/primes.py
@@ -20,6 +20,7 @@ AUTHORS:
# ****************************************************************************
from sage.rings.integer_ring import ZZ
+from sage.rings.semirings.non_negative_integer_semiring import NN
from sage.rings.infinity import infinity
from .set import Set_generic
from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets
@@ -842,16 +843,22 @@ class Primes(Set_generic, UniqueRepresentation):
sage: P.intersection(ZZ) == P
True
+ sage: P.intersection(NN) == P
+ True
sage: P.intersection(RR)
Traceback (most recent call last):
...
NotImplementedError: object does not support iteration
+ sage: P = Primes(modulus=0, classes=range(30))
+ sage: P.intersection(reversed([13, 7, 11]))
+ Finite set of prime numbers: 11, 13, 17, 19
+
.. SEEALSO::
:meth:`complement_in_primes`, :meth:`union`
"""
- if other is ZZ:
+ if other is ZZ or other is NN:
return self
if isinstance(other, Primes):
modulus = self._modulus.lcm(other._modulus)
@@ -862,15 +869,25 @@ class Primes(Set_generic, UniqueRepresentation):
if not b or x in other}
exceptions.update((x, b) for x, b in other._exceptions.items()
if not b or x in self)
- elif self.is_finite():
- modulus = 1
- classes = []
- exceptions = {x: True for x in self if x in other}
else:
- # we try to enumerate the elements of "other"
modulus = 1
classes = []
- exceptions = {x: True for x in list(other) if x in self}
+ if isinstance(other, range) and other.step == 1:
+ exceptions = {}
+ x = other.start - 1
+ while True:
+ x = self.next(x)
+ if x >= other.stop:
+ break
+ exceptions[x] = True
+ elif self.is_finite() and hasattr(other, "__contains__"):
+ # this would not work reliably if ``other`` does not
+ # implement ``__contains__``, because ``x in other``
+ # then consumes ``other``
+ exceptions = {x: True for x in self if x in other}
+ else:
+ # if other is infinite this will loop forever
+ exceptions = {x: True for x in other if x in self}
return Primes(modulus, classes, exceptions)
def union(self, other):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
I just added an additional check to avoid infinite loops if we know in advance that other is not finite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool!
|
Thanks! |
sagemathgh-41122: Subsets of primes defined by congruence conditions We implement subsets of primes given by congruence conditions and operations on those sets. Here is a short demo. ``` sage: P = Primes(modulus=4); P Set of prime numbers congruent to 1 modulo 4: 5, 13, 17, 29, ... sage: P[:10] [5, 13, 17, 29, 37, 41, 53, 61, 73, 89] sage: P.next(500) 509 sage: for p in P: ....: if p > 50: break ....: print(p) 5 13 17 29 37 41 ``` ``` sage: P = Primes(modulus=4); P Set of prime numbers congruent to 1 modulo 4: 5, 13, 17, 29, ... sage: Q = Primes(modulus=4, classes=[3]) Set of prime numbers congruent to 3 modulo 4: 3, 7, 11, 19, ... sage: PQ = P.union(Q) sage: PQ Set of all prime numbers with 2 excluded: 3, 5, 7, 11, ... sage: PQ == Primes().exclude(2) True ``` Cc @FloFuer ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. URL: sagemath#41122 Reported by: Xavier Caruso Reviewer(s): Martin Rubey, Xavier Caruso
We implement subsets of primes given by congruence conditions and operations on those sets.
Here is a short demo.
Cc @FloFuer
📝 Checklist