Vector addition of elements behaving weird

Question

Before I go into the problem description, note that the datatype 'factrep' is merely a typedef of vector<int>.

The problem

Given two factrep objects f1 & f2, passed as arguments into the factrep function mult, I expect the returned factrep object result to be a representation of the multiplied vectors.

Also note that the actual arithmetic operation performed on the vectors' elements individually are addition: this is due to the fact that the vectors are stored in the form of primefactor exponents (element 0 = prime 2 to the power of whatever is stored, element 1 = prime 3 to the power of whatever is stored). Adding the elements as such should therefore be correct, but my results are not coherent. I will post the results in an appendix.

Here is the relevant function

factrep mult(factrep f1, factrep f2)
{
    factrep a;

    for(unsigned int i=0;  i <f1.size() && f2.size(); i++) a.push_back(0);

    for(int i = 0; i < a.size(); i++)
    {
        a[i] += f1[i]+f2[i];
    }

    return a;
}

And here is the full code including the debugging console prints

#include "facth_new.h"
#include <iostream>
#include <cmath>
#include <climits>
#include <vector>

using namespace std;

std::vector<int> eratosthenes(int n) {
    std::vector<int> result = std::vector<int>();
    if (n < 2) {
        return result;
    }

    // initialize the vector
    std::vector<bool> input(n + 1, true);

    // calculate the upper limit as the square root of
    // of N. all composite numbers <= N must have a
    // factor <= sqrt(N)
    int sqrtN = (int)sqrt(n);

    // iterate from 2 up to the square root of N
    for (int i = 2; i <= sqrtN; i ++) {
        if (! input[i]) {
            // i is a proven composite number,
            // all its multiples have been
            // marked not prime by all its prime factors by now.
            continue;
        }

        // as an optimization, all multiples *less* than
        // the square of i are already marked, (they have
        // another prime factor less than i), so we can start
        // from the square which is the smallest composite
        // not yet marked
        for (int j = i * i; j <= n; j += i) {
            input[j] = false;
        }
    }

    // as n >= 2, then add 2 here
    result.push_back(2);

    // and check only odd numbers here,
    // no other even number can be set
    for (int i = 3; i <= n; i += 2) {
        if (input[i]) {
            result.push_back(i);
        }
    }

    return result;
}


factrep primfact(int n){
    factrep a;

    int m; // still to factorize number

    m=n;

    // continue until nothing to factorize
    for(int i = 0; m != 1; i++)
    {
        a.push_back(0);
        while(m % primes[i] == 0){

            m=m/primes[i];
            a.at(i)++;

        }

    }

    return a;
}

factrep mult(factrep f1, factrep f2)
{
    factrep a;

    for(unsigned int i=0;  i <f1.size() && f2.size(); i++) a.push_back(0);

    for(int i = 0; i < a.size(); i++)
    {
        a[i] += f1[i]+f2[i];
    }

    return a;
}

factrep div(factrep f1, factrep f2)
{
    factrep result;

    for(int i=0; i<f1.size(); i++){
        result.push_back(f1[i]-f2[i]);
    }

    return result;
}

double getval(factrep f)
{
    double result = 1;

    for(unsigned int i = 0; i < f.size(); i++)
    {

        result *= pow(primes[i],f[i]);

    }

    return result;
}

vector<int> primes;

int main() {

    primes = eratosthenes(71);
    cout << "The prime numbers are:\n";

    for(unsigned i = 0; i != primes.size(); i++)
        cout << primes[i] << '\n';

    int num[] = {1, 17, 54, 10, 36, 63, 20, 25};
    factrep f[8];


    for(int i = 0; i != 8; i++) {
        f[i] = primfact(num[i]);
    }

    for(int i = 0; i != 8; i++) {
        cout << '\n' << num[i] << " is factorized as:\nPrime     Exponent\n";
        bool agree = true;
        for(unsigned j = f[i].size(); j < f[i].size(); j++) {
            if((f[i])[j] != 0) {
                agree = false;
            }
        }
        for(unsigned j = 0; j != min(f[i].size(), f[i].size()); j++) {
            if((f[i])[j] != 0) {
                cout << primes[j] << "     " << (f[i])[j] << '\n';
            }
        }
        if(!agree) {
            for(unsigned j = f[i].size(); j != f[i].size(); j++) {
                if((f[i])[j] != 0) {
                    cout << primes[j] << "     " << (f[i])[j] << '\n';
                }
            }
        }
    }

    for(int i = 0; i != 8; i++) {
        cout << "getval of " << num[i] << " is " << getval(f[i]) << "\n\n";
    }

    for(int i = 0; i != 8; i++) {
        for(int j = i; j != 8; j++) {
            factrep res = mult(f[i], f[j]);
            cout << num[i] << " multiplied by " << num[j] << " is " << getval(res) << "\n";
        }
    }
    for(int i = 0; i != 8; i++) {
        for(int j = i; j != 8; j++) {
            factrep res = div(f[i], f[j]);
            cout << num[i] << " divided by " << num[j] << " is " << getval(res) << "\n";
        }
    }

}

Finally, my debugging console prints, errors occurring at the multiplication section:

The prime numbers are:
2
3
5
7
11
13
17
19
23
29
31
37
41
43
47
53
59
61
67
71

1 is factorized as:
Prime     Exponent

17 is factorized as:
Prime     Exponent
17     1

54 is factorized as:
Prime     Exponent
2     1
3     3

10 is factorized as:
Prime     Exponent
2     1
5     1

36 is factorized as:
Prime     Exponent
2     2
3     2

63 is factorized as:
Prime     Exponent
3     2
7     1

20 is factorized as:
Prime     Exponent
2     2
5     1

25 is factorized as:
Prime     Exponent
5     2
getval of 1 is 1

getval of 17 is 17

getval of 54 is 54

getval of 10 is 10

getval of 36 is 36

getval of 63 is 63

getval of 20 is 20

getval of 25 is 25

1 multiplied by 1 is 1
1 multiplied by 17 is 1
1 multiplied by 54 is 1
1 multiplied by 10 is 1
1 multiplied by 36 is 1
1 multiplied by 63 is 1
1 multiplied by 20 is 1
1 multiplied by 25 is 1
17 multiplied by 17 is 289
17 multiplied by 54 is 9.12788e+172
17 multiplied by 10 is 1.69035e+172
17 multiplied by 36 is 3.04263e+173
17 multiplied by 63 is 1.06492e+173
17 multiplied by 20 is 2.36649e+173
17 multiplied by 25 is 2.95811e+173
54 multiplied by 54 is 2916
54 multiplied by 10 is 108
54 multiplied by 36 is 1944
54 multiplied by 63 is 486
54 multiplied by 20 is 216
54 multiplied by 25 is 54
10 multiplied by 10 is 100
10 multiplied by 36 is 9000
10 multiplied by 63 is 90
10 multiplied by 20 is 200
10 multiplied by 25 is 250
36 multiplied by 36 is 1296
36 multiplied by 63 is 324
36 multiplied by 20 is 144
36 multiplied by 25 is 36
63 multiplied by 63 is 3969
63 multiplied by 20 is 61740
63 multiplied by 25 is 540225
20 multiplied by 20 is 400
20 multiplied by 25 is 500
25 multiplied by 25 is 625
1 divided by 1 is 1
1 divided by 17 is 1
1 divided by 54 is 1
1 divided by 10 is 1
1 divided by 36 is 1
1 divided by 63 is 1
1 divided by 20 is 1
1 divided by 25 is 1
17 divided by 17 is 1
17 divided by 54 is 0
17 divided by 10 is 0
17 divided by 36 is 0
17 divided by 63 is 0
17 divided by 20 is 0
17 divided by 25 is 0
54 divided by 54 is 1
54 divided by 10 is 27
54 divided by 36 is 1.5
54 divided by 63 is 6
54 divided by 20 is 13.5
54 divided by 25 is 54
10 divided by 10 is 1
10 divided by 36 is 0.277778
10 divided by 63 is 1.11111
10 divided by 20 is 0.5
10 divided by 25 is 0.4
36 divided by 36 is 1
36 divided by 63 is 4
36 divided by 20 is 9
36 divided by 25 is 36
63 divided by 63 is 1
63 divided by 20 is 3.15
63 divided by 25 is 0.36
20 divided by 20 is 1
20 divided by 25 is 0.8
25 divided by 25 is 1

You should pass those `factrec` objects by reference, not by value. — PaulMcKenzie, Mar 08 '15 at 16:50
two={1}, three={0,1}, mult(two,three) will cause out-of-bounds access to two[1]. always fully parenthesize your expressions while you're learning (i.e. `i — Will Ness, Mar 08 '15 at 17:10

score 1 · Answer 1 · answered Mar 08 '15 at 16:32

1

the condition i <f1.size() && f2.size() should rather be written i <f1.size() && i < f2.size()

answered Mar 08 '15 at 16:32

W.F.

13,888
2
34
81

Thanks for pointing out this flaw. However, when I edit this condition I still end up with the same results as posted. – Syncretic Mar 08 '15 at 18:40

score 1 · Answer 2 · answered Mar 08 '15 at 16:33

1

Your for loop's condition :

for(unsigned int i=0;  i <f1.size() && f2.size(); i++)

should be :

for(unsigned int i=0;  i<f1.size() && i<f2.size(); i++)

This will mean that i loops from 0 to min{f1.size(), f2.size()}

answered Mar 08 '15 at 16:33

a_pradhan

3,285
1
18
23

I understand this condition logic flaw, but as I edit it I get the same results. – Syncretic Mar 08 '15 at 18:39

score 1 · Answer 3 · answered Mar 08 '15 at 16:49

As the others have stated, the problem lies in your loop's condition. Beyond that I notised a couple of things that you could change to make your code more logical.

You have the following two loops

for(unsigned j = f[i].size(); j < f[i].size(); j++) { if((f[i])[j] != 0) { agree = false; } }

for(unsigned j = f[i].size(); j != f[i].size(); j++) { if((f[i])[j] != 0) { cout << primes[j] << " " << (f[i])[j] << '\n'; } }

In both of these you have you're initialising j with f[i].size() and then setting the condition to end the loop if j==f[i].size(). Since j is already f[i].size(), this will break after the first iteration. In that case, you don't need the loop at all, and could instead just have your if statements.

Vector addition of elements behaving weird

3 Answers3