Project Euler #23, can't find the issue in program

Question

Link : http://projecteuler.net/problem=23

A perfect number is a number for which the sum of its proper divisors is exactly equal to the number. For example, the sum of the proper divisors of 28 would be 1 + 2 + 4 + 7 + 14 = 28, which means that 28 is a perfect number.

A number n is called deficient if the sum of its proper divisors is less than n and it is called abundant if this sum exceeds n.

As 12 is the smallest abundant number, 1 + 2 + 3 + 4 + 6 = 16, the smallest number that can be written as the sum of two abundant numbers is 24. By mathematical analysis, it can be shown that all integers greater than 28123 can be written as the sum of two abundant numbers. However, this upper limit cannot be reduced any further by analysis even though it is known that the greatest number that cannot be expressed as the sum of two abundant numbers is less than this limit.

Find the sum of all the positive integers which cannot be written as the sum of two abundant numbers.

---

The answer to the problem is 4179871, my program shows 3797954.

First and foremost, I make a function to fill array abundant[ ] with all abundant numbers below 28124. This works perfectly fine, because I googled abundant numbers and they match up exactly with my array.

Secondly, I have another array with all numbers 1-28123, I assume ALL of them "cannot be written as the sum of two abundant numbers." These are all written into array hold[ ].

Finally, I get rid of the numbers that CAN be written as the sum of two abundant numbers, by adding all numbers in abundant[ ] with all numbers in abundant[ ], and setting that value of hold[ ] to 0. (hold[abundant[0 to n]+abundant[0 to n]] = 0) Add all remaining numbers in hold[ ], I only get 3797954

I know this program isn't very efficient, since it adds all abundant numbers to all abundant numbers, but it should work just fine. What's wrong with it?

---

#include <iostream>
#include <cmath>

using namespace std;

int hold[28124];
int abundant[7000]; //hold abundant numbers, there are only 6919 abundant numbers below 28123

bool abundance(int x){  //returns true if x is abundant
    int counter = 1;    //holds "proper divisors" of numbers, default by 1 because every int is divisible by 1
    for (int i = 2; i < sqrt(x); i++){   //finds all divisors 2 - sqrt(n)
        if (x % i == 0){
            counter += i;
            counter += x / i;
        }
    }
    int y = sqrt(x);   
    if (x % y == 0){   //adds sqrt(n) if its modulus to n is 0
            counter += sqrt(x);
        }
    if (counter > x){
        return true;
    } else {
        return false;
    }
}

int main()
{
    int counter = 0;
    for (int i = 0; i < 28124; i++){ //assumes every number cannot be written as the sum of two abundant numbers,
        hold[i] = i;                 //goes up to 28123 because "it can be shown that all integers greater
    }                                //than 28123 can be written as the sum of two abundant numbers." - project euler
    for (int j = 10; j < 28124; j++){ 
        if (abundance(j) == true){  //copies all abundant numbers up to 28123 to abundant[]
            abundant[counter] = j;
            counter++;
        }
    }

    for (int m = 0; m < counter; m++){  //adds all numbers in abundant[], with all numbers in abundant[]
        for (int n = 0; n < counter; n++){
            if (abundant[m]+abundant[n] < 28124){
                hold[abundant[m]+abundant[n]] = 0; //sum of the abundant numbers in hold[] is set to 0
            } //hold[] now holds all numbers that cannot be written as the sum of 2 abundant numbers
        }
    }
    int counter2 = 0;
    for (int x = 0; x < 28124; x++){
        counter2 += hold[x];
    }
    cout << counter2 << endl;

}

Alphitomancy

Not a fix - but you can speed things up by setting y = sqrt(x) once, and then replacing other instances of sqrt(x) with y. sqrt is a fairly expensive function... – Demivolt

Answer 1

The problem is in your abundance function, specifically this part:

int y = sqrt(x);   
if (x % y == 0){   //adds sqrt(n) if its modulus to n is 0
    counter += sqrt(x);
}

x % (int)sqrt(x) == 0 does not imply that sqrt(x) is an integer. A simple counterexample is 2. sqrt(2) is about 1.414, or just 1 as an integer. But 2 % 1 == 0, even though it isn't the square root.

So to fix your code, change that part to:

int y = sqrt(x);   
if (y * y == x){   //adds sqrt(n) if sqrt(n) is an integer
    counter += y;
}

And you will get the correct results.

Plumate

+1 for significantly speeding up the code as well (using y instead of sqrt(x) when the counter is incremented). – Demivolt

I really appreciate it, this has been driving me crazy – Alphitomancy

Answer 2

You might take a look at the code shown here - if only because it gets the right answer. Without copying what was done there, you could for example confirm whether the problem is in your abundant number generator or in the other part. It might help you figure out where you are going wrong.

Demivolt

That program seems pretty identical to mine, I can't really see the difference, besides him having a more efficient way to find abundant sums. I tried it but it doesn't change my program's result – Alphitomancy

"I tried it but it doesn't change my program's result": Are you saying your code got the same list of abundant numbers as the one I linked you to? If so, then @Blender's answer can't be right... but I suspect he is (his fix should reduce the number of abundant numbers). – Demivolt

I didn't check all 6919 of them, but the first few hundred seem to be identical. I guess I need to be less lazy – Alphitomancy

When you have a computer to check things for you, you might as well be thorough. Project Euler is all about learning - I think you just did that. :-) – Demivolt

Answer 3

my_set = set([i for i in range(1, 28123)])
print ('original sum',sum(my_set))
my_list = list(my_set)
my_l1 =[]
for k in range(12, int(my_list[-1]/2+1)):
    a = 0
    for j in range(1, int(k/2+1)):
        if k%j == 0:
            a += j
        
    if a > k:
        my_l1.append(k)
        my_set.remove(k*2)
#Calculating the sum of all the numbers which can be written as the sum of two abundant numbers   
l = 0
my_l2 = set([])
for d in my_l1:
    l += 1
    k = l
    while k < len(my_l1):
        a_s = d + my_l1[k]
        my_l2.add(a_s)
        k += 1
my_set.difference_update(my_l2)
print ('sum of all abbundant numbers:',sum(my_set))

here's my code for problem23 Project Euler, what's wrong with this? I'm not concerned with the runtime for now, I just want the correct answer.

python

Homogenize