-1

I just like to do simple recursion. For a number if it is even then it'll approach to (number/2) and if odd then to (3*number+1). How many time it'll occur to reach 1.

for 10
10-> 5-> 16-> 8-> 4 -> 2 ->1
total process 6

long arr[10000];
long dp(int n)  
{ 
    if(n==2|| n==1) return 1;
    if(arr[n]) return arr[n];

    if(n%2==0) return arr[n]=1+dp(n/2);
    else if(n%2==1) return arr[n]=1+dp(3*n+1);
    return arr[n];
}

I've created function like this one and for some input like 999 or 907 causes segmentation fault.
I wanna know why?

And if I increase array size then its output correctly.
I wanna know why too?

Why its depending on array size as I've taken long as array element data type so it should output correctly for those input?

bwDraco
  • 2,514
  • 2
  • 33
  • 38
LogicalAnt
  • 907
  • 3
  • 12
  • 28
  • 6
    Those numbers probably trigger enough 3n+1 steps to end up above 10000, making you access memory outside the array. You should probably start by making "pure" recursion work (i.e. write dp(n) instead of arr[n] and get rid of the array entirely) then later on worry about a safe way of caching intermediate results. That aside, this question probably belongs on SO. – Ixrec Jan 04 '15 at 22:58
  • I was always wondering, does the OP really knows that the post is migrated? – gsamaras Jan 04 '15 at 23:47
  • A much better solution is to use a [map](http://www.cplusplus.com/reference/map/map/), so that you can easily store and look up results without using a lot of memory. – bwDraco Jan 05 '15 at 01:28
  • yea i am concerned :/ @G.Samaras – LogicalAnt Jan 05 '15 at 15:40

5 Answers5

3

with 999, you reach 11392

with 907, you reach 13120

and those numbers are out of bound.

Live example

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • I'm getting a stack overflow (no pun intended) with 999 as input, which will result in a segmentation fault. Changing the array size to 20000 does not help. – bwDraco Jan 04 '15 at 23:51
  • Not working for me whether with GCC or VC++. The program is overflowing the call stack. – bwDraco Jan 05 '15 at 00:00
  • Never mind. I moved the array into the function rather than declaring it as a global variable as in the question, that's why the stack is overflowing. Now I get "Access violation writing location..." – bwDraco Jan 05 '15 at 00:17
  • Your implementation was really helpful. @Jarod42 – LogicalAnt Jan 05 '15 at 16:21
1

You are indexing the array out of bounds

  • For the inputs you are using, the variable n will exceed the array size 10000 during execution. This causes the program to access memory beyond its boundaries, resulting in a segmentation fault.

  • If you're trying to memoize the function, I'd suggest using std::map instead of a fixed array. This is an associative array which stores key-value pairs—in this case, each memoized input-output pair—and can quickly retrieve the value associated with a given key. Maps are ideally suited for this application as they can store this data using only as much memory as is actually necessary, automatically growing as needed.

  • You could also use std::vector, though this is not recommended. Vectors are like arrays, but they can be resized dynamically, avoiding the problem of indexing out of bounds. The drawback with this approach is that for certain inputs, a very large amount of memory may be required, possibly several gigabytes. If the program is compiled to a 32-bit binary rather than to a 64-bit binary, the program may crash at runtime when it fails to allocate enough memory for the vector.

Implementation using map

#include <iostream>
#include <map>

using namespace std;

long long dp(unsigned long long);

int main() {
    unsigned long long n;

    while(true) {
        cout << "Enter a number, or 0 to exit: ";
        cin >> n;

        if(!cin) {
            cin.clear();
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            cerr << "Invalid input, please try again." << endl;
            continue;
        }

        if(n == 0)
            return 0;
        else
            cout << dp(n) << endl;
    }

    return 0; // Unreachable
}


long long dp(unsigned long long n) {
    static map<long long, long long> memo;

    if(n == 2 || n == 1) return 1;
    if(memo[n]) return memo[n];

    if(n % 2 == 0) return memo[n] = 1 + dp(n / 2);
    else if(n % 2 == 1) return memo[n] = 1 + dp(3 * n + 1);
    return memo[n];
}

Implementation using vector

#include <iostream>
#include <vector>

using namespace std;

long long dp(unsigned long long);

int main() {
    unsigned long long n;

    while(true) {
        cout << "Enter a number, or 0 to exit: ";
        cin >> n;

        if(!cin) {
            cin.clear();
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            cerr << "Invalid input, please try again." << endl;
            continue;
        }

        if(n == 0)
            return 0;
        else
            cout << dp(n) << endl;
    }

    return 0; // Unreachable
}


long long dp(unsigned long long n) {
    static vector<long long> arr;

    if(arr.size() <= n)
        arr.resize(n + 1);

    if(n == 2 || n == 1) return 1;
    if(arr[n]) return arr[n];

    if(n % 2 == 0) return arr[n] = 1 + dp(n / 2);
    else if(n % 2 == 1) return arr[n] = 1 + dp(3 * n + 1);
    return arr[n];
}
bwDraco
  • 2,514
  • 2
  • 33
  • 38
0

The segmentation fault comes from overflowing the array. How about you do something like the following?

void rec(int n, int* steps) {
  ++(*steps);
  printf("%d\n", n);
  // termination step if 'n' equals 1
  if(n == 1)
    return;
  if (n % 2 == 0)
    rec(n/2, steps);
  else
    rec(3*n+1, steps);
}

int main(void) {
  int steps = 0;
  rec(10, &steps);
  printf("Steps = %d\n", steps);
  return 0;
}

Output:

10
5
16
8
4
2
1
Steps = 7

The code presented here agrees (of course) with Jarod's answer.

Community
  • 1
  • 1
gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • OP wants to do some memorization to speed up computation with successive calls. – Jarod42 Jan 04 '15 at 23:50
  • You saw that from the OP's code right? The question does not mentions something like that @Jarod42. Or I missed that? – gsamaras Jan 04 '15 at 23:51
  • @Jarod42 you might have a point here. However, I think you agree that there is a really small chance that this is not the case. As a result I will leave the answer as is, just for the OP to take a look. If needed, I can remove it later. :) If you think otherwise, please let me know. – gsamaras Jan 05 '15 at 00:06
  • 2
    @Jarod42: I think you meant [memoization](https://en.wikipedia.org/wiki/Memoization). – bwDraco Jan 05 '15 at 00:55
  • @DragonLord: yeah, unfortunately, I use a spell checker which doesn't know this word :/. – Jarod42 Jan 05 '15 at 08:30
  • I've done some memorization cause I know its a better approach. If somethings there better than that then its welcomed. – LogicalAnt Jan 05 '15 at 15:51
  • I see. @Jarod42 you deserve a +1 for seeing this. As a matter of fact I am really happy a question (http://stackoverflow.com/questions/26716255/why-does-this-program-print-forked-4-times/26716300#26716300) with my best answer ever is opened, so if you want me to delete this let me know. – gsamaras Jan 05 '15 at 17:58
-1

You are overflowing the array because the sequence value exceeds 9999.

kevin cline
  • 2,608
  • 2
  • 25
  • 38
-1

If all you want to know is the number of processes that it takes, you should not be using array storage.

int numProcesses = 0;

int hailstone(int n) {

    if (n == 1) {
        return 1;
    } // base case

    ++numProcesses; // if it wasn't the base case the method will do some work so increment numProcesses

    if (n % 2 == 0) {
        return hailstone(n / 2);
    } // n is even
    else {
        return hailstone(3 * n + 1);
    } // n is odd
}

This is untested but I think it should work, and after it finally returns numProcesses should equal the number of times the method was called (so long as it was not called with a parameter of 1).

j_v_wow_d
  • 511
  • 2
  • 11