Mantis - Squeak
Viewing Issue Advanced Details
2118 System minor always 10-24-05 16:10 02-13-06 14:41
sumi  
 
normal  
closed 3.9  
fixed  
none    
none 3.9  
0002118: Integer class >> #primesUpTo: omits the largest prime number when the arg is larger than 25001
| nn | nn := (2 raisedTo: 17) - 1. (Integer primesUpTo: nn) last = nn " => false "

but, it should be true.

It seems that Integer class >> #largePrimesUpTo:do: has any bugs.

Notes
(0002955)
sam   
10-24-05 16:29   
The "- 1" in "limit := max asInteger - 1" seems erroneous. Also, by reading and testing the code, I noticed that in every case it returned the primes up to 11, so I added an extra test for this to call #primesUpTo:do: instead of #largePrimesUpTo:do: in this special case.

I'll attach a proposed changeset (no Monticello setup where I am right now).
(0002967)
andreas   
10-25-05 09:15   
Depends on your definition of "up to". The one I used was the one from Stream which implements #upTo: by "answering a subcollection from the current access position to the occurrence (if any, but not inclusive) of anObject in the receiver". Notice the "but not inclusive".
(0002968)
sumi   
10-25-05 10:20   
Integer primesUpTo: 5 " => #(2 3 5) "

seems to include 5, doesn't it?
(0002969)
andreas   
10-25-05 11:52   
Sure does. But does that invalidate my argument? ;-)
(0002970)
sam   
10-25-05 11:54   
Andreas: what is *certain* at this time is that the behaviour is bogus and inconsistent. I would propose to apply my CS first then maybe to change the code so that the meaning of upTo is fixed if needed.

Otherwise, nothing will get fixed and the behaviour will stay as it is.
(0002979)
andreas   
10-25-05 21:44   
All I'm asking for is a justification, any justification for why primesUpTo: should indeed include the argument. You say it's inconsistent, and I agree, but that doesn't necessarily mean it must be changed in the way you propose. I have given an argument for doing it the way I decided to implement the method (consistency with naming conventions in Stream) and if you have an argument for why the method should include the argument (if prime), I'd like to hear it.
(0002982)
sumi   
10-26-05 05:49   
sam,
Thanks your supplementary explanation.

andreas,
As sam said, I think that the problem is the dependence of behavior of #primesUpTo: upon its argument (larger than 25000 or not). Definition of "up to" is the same of yours, not inclusive of arg (even if it is prime, in this case). I apologize that the title of this report is not proper. The bug is in #primesUpTo:do:, but not in #largePrimesUpTo:do:.

So, the Integer class >> #primesUpTo:do: of

    1 to: limit do: [:i | ...

should be

    1 to: limit - 1 do: [:i | ...
(0003716)
MarcusDenker   
02-08-06 20:56   
I added tests

testPrimesUpTo

    "upTo: semantics means 'non-inclusive'"
    primes := Integer primesUpTo: 5.
    self assert: primes = #(2 3).
    
    "this test is green for nn>25000, see #testLargePrimesUpTo"
    nn := 5.
    self deny: (Integer primesUpTo: nn) last = nn.
    self assert: (Integer primesUpTo: nn + 1) last = nn.

and

testLargePrimesUpTo

    | nn |
    nn := (2 raisedTo: 17) - 1.
    self deny: (Integer primesUpTo: nn) last = nn.
    self assert: (Integer primesUpTo: nn + 1) last = nn.
    

with testPrimesUpTo failing in 3.9a
(0003777)
MarcusDenker   
02-13-06 14:41   
fixed