CODEWITHSUNDEEP
c++collatz
PictureMeAndYou
PictureMeAndYou

Reputation: 533

Stuck in an infinite loop? (Maybe)

I am trying to complete Project Euler Problem 14 in c++ and I am honestly stuck. Right now when I run the problem it gets stuck at So Far: the number with the highest count: 113370 with the count of 155 So Far: the number with the highest count but when I try changing the i value to over 113371 it works. What is going on??

The question is:

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?

#include<stdio.h>
int main() {
    int limit = 1000000;
    int highNum, number, i;
    int highCount = 0;
    int count = 0;
    for( number = 13; number <= 1000000; number++ )
    {
        i = number;
        while( i != 1 ) {
            if (( i % 2 ) != 0 ) {
                i = ( i * 3 ) + 1;
                count++;
            }
            else {
                count++;
                i /= 2;
            }
        }
        count++;
        printf( "So Far: the number with the highest count: %d with the count of %d\n",
                     number, count );
        if( highCount < count ) {
            highCount = count;
            highNum = number;
        }
        count = 0;
        //break;
    }
    printf( "The number with the highest count: %d with the count of %d\n",
            highNum, highCount );
}

Upvotes: 4

Views: 383

Answers (4)

Deduplicator
Deduplicator

Reputation: 45674

Remember all intermediate results (up to some suitably high number).
Also, use a big-enough type:

#include <stdio.h>

static int collatz[4000000];
unsigned long long collatzmax;

int comp(unsigned long long i) {
  if(i>=sizeof collatz/sizeof*collatz) {
      if(i>collatzmax)
        collatzmax = i;
      return 1 + comp(i&1 ? 3*i+1 : i/2);
  }
  if(!collatz[i])
      collatz[i] = 1 + comp(i&1 ? 3*i+1 : i/2);
  return collatz[i];
}

int main() {
  collatz[1] = 1;
  int highNumber= 1, highCount = 1, c;
  for(int i = 2; i < 1000000; i++)
    if((c = comp(i)) > highCount) {
      highCount = c;
      highNumber = i;
    }
  printf( "The number with the highest count: %d with the count of %d\n",
        highNumber, highCount );
  printf( "Highest intermediary number: %llu\n", collatzmax);
}

On coliru: http://coliru.stacked-crooked.com/a/773bd8c5f4e7d5a9

Variant with smaller runtime: http://coliru.stacked-crooked.com/a/2132cb74e4605d5f

The number with the highest count: 837799 with the count of 525
Highest intermediary number: 56991483520

BTW: The highest intermediary encountered needs 36 bit to represent as an unsigned number.

Upvotes: 3

isekaijin
isekaijin

Reputation: 19762

Don't recompute the same intermediate results over and over!

Given

typedef std::uint64_t num;  // largest reliable built-in unsigned integer type

num collatz(num x)
{
    return (x & 1) ? (3*x + 1) : (x/2);
}

Then the value of collatz(x) only depends on x, not on when you call it. (In other words, collatz is a pure function.) As a consequence, you can memoize the values of collatz(x) for different values of x. For this purpose, you could use a std::map<num, num> or a std::unordered_map<num, num>.

For reference, here is the complete solution.

And here it is on Coliru, with timing (2.6 secs).

Upvotes: 0

Jarod42
Jarod42

Reputation: 217850

With your algorithm, you compute several time identical series. you may cache result for previous numbers and reuse them.

Something like:

void compute(std::map<std::uint64_t, int>& counts, std::uint64_t i)
{
    std::vector<std::uint64_t> series;
    while (counts[i] == 0) {
        series.push_back(i);
        if ((i % 2) != 0) {
            i = (i * 3) + 1;
        } else {
            i /= 2;
        }
    }
    int count = counts[i];
    for (auto it = series.rbegin(); it != series.rend(); ++it)
    {
        counts[*it] = ++count;
    }
}

int main()
{
    const std::uint64_t limit = 1000000;

    std::map<std::uint64_t, int> counts;
    counts[1] = 1;
    for (std::size_t i = 2; i != limit; ++i) {
        compute(counts, i);
    }
    auto it = std::max_element(counts.begin(), counts.end(),
        [](const std::pair<std::uint64_t, int>& lhs, const std::pair<std::uint64_t, int>& rhs)
        {
            return lhs.second < rhs.second;
        });
    std::cout << it->first << ":" << it->second << std::endl;
    std::cout << limit-1 << ":" << counts[limit-1] << std::endl;
}

Demo (10 seconds)

Upvotes: 0

Anton Savin
Anton Savin

Reputation: 41321

You are getting integer overflow. Update your code like this and see it yourself:

if (( i % 2 ) != 0 ) {
    int prevI = i;
    i = ( i * 3 ) + 1;
    if (i < prevI) {
        printf("oops, i < prevI: %d\n", i);
        return 0;
    }
    count++;
}

You should change the type of i to long long or unsigned long long to prevent the overflow.

(And yes, cache the intermediate results)

Upvotes: 3

Related Questions