fix incorrect rounding in sieve of Eratosthenes

[LizardWizzard]

Hello!

Just discovered that there is an error in sieve of Eratosthenes. Value from sqrt was rounded to floor but it has to be rounded to ceil.
Simple example: implementation without provided fix will count 9 as prime number while finding prime numbers less than 10. So this is incorrect and should be fixed.

Thanks!

[obelisk0114]

I think it should be round(sqrt(n)) + 1

[LizardWizzard]

Your option is also fine but it will produce an extra check in case when fractional part of square root is greater than 0.5. So for example when finding prime numbers less than 14 square root of 14 equals 3.7416... and for ceil and round without an extra plus one there will be 4 checks, but for round with +1 there will be 5 iterations

[cclauss]

OPTIONAL: Do you feel like adding a doctest so that we can automatically catch errors in the future?

[brunohadlich]

@LizardWizzard your approach works fine for entry 10 but in case user enters 9 or 25 these numbers 9 and 25 will be incorrectly considered as prime numbers.

As mentioned by @obelisk0114 round(sqrt(n)) + 1 is the correct solution, this approach solves both entries, 9 and 10.

[brunohadlich]

@LizardWizzard I did some changes to this file, added doctest, reduced lines length to 79 and executed python black to standardize code format, feel free to pursuit your idea of reducing necessary iterations, to check if your implementation is working you can run in a shell python3 -m doctest -v finding_primes.py

"""
The sieve of Eratosthenes is an algorithm used to find prime numbers, less
than or equal to a given value.
"""
from math import sqrt


def SOE(n):
    """Returns a list with all prime numbers up to n.

    >>> SOE(50)
    [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47]
    >>> SOE(25)
    [2, 3, 5, 7, 11, 13, 17, 19, 23]
    >>> SOE(10)
    [2, 3, 5, 7]
    >>> SOE(9)
    [2, 3, 5, 7]
    >>> SOE(2)
    [2]
    >>> SOE(1)
    []
    """
    # Need not check for multiples past the square root of n
    check = round((sqrt(n))) + 1

    # Set every index to True except for index 0 and 1
    sieve = [False if i < 2 else True for i in range(n + 1)]

    primes = []

    for i in range(2, check):
        if sieve[i] == True:  # If i is a prime
            # Step through the list in increments of i(multiples of the prime)
            for j in range(i + i, n + 1, i):
                sieve[j] = False  # Sets every multiple of i to False

    for i in range(n + 1):
        if sieve[i] == True:
            primes.append(i)
    return primes


if __name__ == "__main__":
    print(SOE(int(str(input()).strip())), end=" ")

[cclauss]

    for i in range(n + 1):
        if sieve[i] == True:
            primes.append(i)
    return primes
# can be rewritten as
    return primes + [i for i in range(n + 1) if sieve[i]]

[LizardWizzard]

Sorry for late response. Thanks to @brunohadlich for pointing to my mistake directly. I think proposed solution with round(sqrt(n)) + 1 is the most convenient and simple one. Maybe there is more special one which will exclude one extra iteration but I currently have no time to dig into this. @brunohadlich you have proposed solution with working code, will you make a PR yourself or I can somehow help with this (may be update this PR with your code if you want so)?

[brunohadlich]

Yes you can update this PR with the code I proposed, feel free to do that because today and tomorrow I won't have time to help with the project.

[obelisk0114]

int(sqrt(n)) + 1

[github-actions]

Stale pull request message