What is the problem with my code Longest Collatz Sequence?

Question

I have been trying to solve this problem :

The following iterative sequence is defined for the set of positive integers:

n → n/2 (n is even)
n → 3n + 1 (n is odd)

Using the rule above and starting with 13, we generate the following sequence:
13 → 40 → 20 → 10 → 5 → 16 → 8 → 4 → 2 → 1

It can be seen that this sequence (starting at 13 and finishing at 1) contains 10 terms. Although it has not been proved yet (Collatz Problem), it is thought that all starting numbers finish at 1.

Which starting number, under one million, produces the longest chain?

NOTE: Once the chain starts the terms are allowed to go above one million.

and I implemented the below code, but this doesn't seem to give me the correct answer. It computes 910107 as the starting number that gives the longest chain, but the answer should be 837799. What's wrong with it?

#include<stdio.h>

int main(void)
{

    int count=1;
    int last_count =0;
    int num=13;
    int temp;
    int Largest_Num=0;
    for(int i=num;i<1000000;i++)
    {
        temp = i;
        while(temp>1)
        {
            if(temp % 2 == 0)
            {
                temp/=2;
            }
            else
            {
                temp =(3*temp)+1;
            }
            count++;   

        }

        if(last_count < count)
        {
            last_count = count;
            Largest_Num = i;
        }

        count =1;
    }
    printf("%d\n",last_count);
    printf("%d",Largest_Num);

    return 0;
}

Answer 1

I get 910107 as the starting number that gives the longest chain but the answer should be 837799

int is too short to manage the large numbers you need for temp, you have overflow

That means your int is on 32 bits, and in that case int use only 31 bits for the positive numbers while you need 32

You can declare temp as an unsigned int (getting 1 bit more than an int because you only need positive numbers), or use a long if it is on 64 bits for you, or a long long to be sure to have at least 64 bits

Answer 2

As @bruno observed in his answer, you are exceeding the capacity of your system's int data type. The problem even hints that this is an issue you might encounter when it says:

NOTE: Once the chain starts the terms are allowed to go above one million.

How much above? That's not clear from the problem, so "how can I determine what data type will be sufficient?" is a question that should immediately spring to mind. Of course, the antecedent question "what data type should I choose?" is one that should always get at least a few moments of genuine attention.

Assuming no prior knowledge of the sequences you intend to compute, and having no clear way to determine reliable upper bounds for their elements in the general case, determining which data type will be sufficient requires actually performing (attempting) the computation and watching for overflow. Alternatively, you can couch it as choosing a data type you think will be sufficient, and verifying as you go along that it in fact is sufficient. If it's not, then you try again with a different data type.

Guarding against overflow is easy in this case. The only variable that needs watching is temp, and its value increases only when you compute temp =(3*temp)+1. You know what the maximum value of type int is (INT_MAX), and it is simple to determine algebraically the maximum value of temp for which that computation does not exceed INT_MAX: it is (INT_MAX - 1) / 3. Thus, you can do this:

#include <assert.h>
#define TEMP_BOUND ((INT_MAX - 1) / 3)

// ...

            if(temp % 2 == 0) {
                temp /= 2;
            } else {
                assert(temp <= TEMP_BOUND);
                temp = (3 * temp) + 1;
            }

If you end up with an assertion failure (as you will when using type int), you can try again with a data type that supports a greater maximum value; just be sure then to update the TEMP BOUND macro appropriately for the new data type.

I should also observe that there is the alternative of performing your computations with an arbitrary-precision data type, such as is available from various third-party libraries (GMP, for example). This will be much slower, though, and the code will be more complex, so for a case like this I do recommend the trial-and-error approach outlined above instead, at least up to the point that you determine that no built-in data type suffices for the problem.

Answer 3

Try This One

#include <stdio.h>
#include <stdlib.h>

int checklongestchain(unsigned int number){
        int count = 0;
        while(number > 1){
                //printf(" %d  for %d\n",number,count);
                if(number % 2 == 0){
                        number = number / 2;
                        count++;
                }else{
                        number = (3 * number) +1;
                        count++;
                }
        }
        return count;
}

int main(int argc, char **argv) {
        unsigned int NUMBER, s;
        unsigned int i;
        int no = 0;
        int maxcount = 0;
        NUMBER = atoi(argv[1]);
        for (i = 1; i < NUMBER; i++) {
                no = checklongestchain(i);
                printf("Chain Length For : %d  for %d\n",no,i);
                if(no > maxcount){
                        maxcount = no;
                        s=i;
                        printf("Found New Largest Chain: %d  for %d\n",maxcount+1,s);
                }
        }
        printf("Found Largest Chain: %d  for %d\n",maxcount+1,s);
        return 0;
}

gcc prog.c -o prog

time ./prog 1000000