0

This piece of code is taken from a bigger code, where it was creating a problem. I am able to reproduce the same error here.

The code requires unsigned long long integers to deal with large values. The requirement is to convert a given binary number (stored in a vector) to a decimal number (unsigned long long). It is supposed to be a simple code. But it turns out the function configuration_to_int_1 produces wrong result for some binary numbers. If you run the code, you will see that in the very last step of the loop, it adds an even and an odd number to give an even output, which is bizarre!! After spending an entire day to understand the root of this problem, I rewrote the function as configuration_to_int_2 where I have just stored the value of pow(2,i) in a long long integer at the beginning and used this variable whenever pow(2,i) was required. This gives me the correct value!!

Can anyone explain to me, what was going wrong in the first case? Thanks in advance. Here is the code:

#include <stdio.h>
#include <bitset>
#include <vector>
#include <iostream>
#include <math.h>

using namespace std;

int main()
{
    unsigned long long hash=9007199256641537;
    int NLL, Nphi;
    NLL=3;
    Nphi=18;
    vector<unsigned short int> basis(3*Nphi,0);
    
    unsigned long long int configuration_to_int_1(vector<unsigned short int> conf, int NLL, int Nphi);
    unsigned long long int configuration_to_int_2(vector<unsigned short int> conf, int NLL, int Nphi);
    void int_to_occupation(unsigned long long int num, int Nphi, int NLL, vector<unsigned short int>& occupation);
    
    int_to_occupation(hash, Nphi, NLL, basis);
    configuration_to_int_1(basis, NLL, Nphi);
    configuration_to_int_2(basis, NLL, Nphi);
    
    
        
    return 0;
}


void int_to_occupation(unsigned long long int num, int Nphi, int NLL, vector<unsigned short int>& occupation)
{
        bitset<64> bb(num);
        for (int r = 0; r < NLL*Nphi; ++r)
        {
                occupation[r] = bb[r];
        }
}



unsigned long long int configuration_to_int_1(vector<unsigned short int> conf, int NLL, int Nphi)
{
        unsigned long long int x=0;
        for(int i=0;i< NLL*Nphi;i++)
        {
                if (conf[i]==1)
                {
                        cout<<x<<" + "<<pow(2,i)<<" = ";
                        x=x+pow(2,i);
                        cout<<x<<endl;
                }
        }

    return x;
}

unsigned long long int configuration_to_int_2(vector<unsigned short int> conf, int NLL, int Nphi)
{
        unsigned long long int x=0,c;
        for(int i=0;i< NLL*Nphi;i++)
        {
                if (conf[i]==1)
                {
                        c=pow(2,i);
                        cout<<x<<" + "<<c<<" = ";
                        x=x+c;
                        cout<<x<<endl;
                }
        }

    return x;
}
Abhishek Anand
  • 121
  • 1
  • 8
  • 3
    I guess it's because of the totally unnecessary usage of `double`s and `pow` in a problem that involves only integers. – Jabberwocky May 27 '22 at 13:36
  • 1
    If you insist using `double`s and `pow`, use your debugger and/or print relevant values at strategic points of your code and find out. – Jabberwocky May 27 '22 at 13:38
  • 2
    [Don't use `pow` for integer powers](https://stackoverflow.com/questions/15851636/why-is-my-integer-math-with-stdpow-giving-the-wrong-answer) – NathanOliver May 27 '22 at 13:39
  • 1
    Change the type of `c` to `double` and see what happens. – Scott Hunter May 27 '22 at 13:39
  • 1
    Hint: powers of 2 can be calculated using the `<<` (shift left) operator. – Jabberwocky May 27 '22 at 13:40
  • 1
    The compiler interprets `x=x+pow(2,i);` as `x=static_cast(static_cast(x) + pow(2, i));`. `double` cannot exactly represent all `unsigned long long` values though. E.g. 2^64-1 (i.e. all bits are 1) cannot be represented resulting in a loss of data because of the conversions. Converting `pow(...)` to `unsigned long long` before adding as done in v2 prevents this issue.Btw: why not just iterate though the vector and during each iteration multiply the current result with 2 and add the current bit?Also better pass the vectors by const refs to avoid unnecessary copying... – fabian May 27 '22 at 13:46
  • 1
    The difference between the two functions is because one is doing `unsigned long long` math, and the other is doing `double` math. – Eljay May 27 '22 at 13:58
  • Thank you for all your answers. I understand the problem in the code. Sorry, I am just a physicist, so I lack some basic understandings of the data structures. – Abhishek Anand May 27 '22 at 14:28
  • @fabian I see. I will implement the changes. Thanks. – Abhishek Anand May 27 '22 at 14:33

0 Answers0