1

I'm writing a program that is to take a number between 1-10 and display all possible ways of arranging the numbers.

Ex input: 3 output:

   1 2 3
   1 3 2
   2 1 3
   2 3 1
   3 1 2
   3 2 1

Whenever I input 9 or 10, the program gives a segmentation fault and dumps the core. I believe the issue is my recursive algorithm is being called too many times. Could someone help point out how I could limit the amount of recursive calls necessary? Here is my current code:

void rearange(int numbers[11], int index, int num, int fact) {

  int temp = numbers[index];
  numbers[index] = numbers[index-1];
  numbers[index-1] = temp;

  int i;
  for (i = 1; i <= num; ++i) // print the current sequence
  {
    printf("%d ", numbers[i]);
  }
  printf("\n");

  fact--;  // decrement how many sequences remain
  index--; // decrement our index in the array

  if (index == 1) // if we're at the beginning of the array
    index = num;    // reset index to end of the array

  if (fact > 0) // If we have more sequences remaining
    rearange(numbers, index, num, fact);    // Do it all again! :D
}

int main() {
  int num, i; // our number and a counter

  printf("Enter a number less than 10: ");
  scanf("%d", &num); // get the number from the user

  int numbers[11]; // create an array of appropriate size
  // fill array
  for (i = 1; i <= num; i++) { // fill the array from 1 to num
    numbers[i] = i;
  }

  int fact = 1; // calculate the factorial to determine
  for (i = 1; i <= num; ++i) // how many possible sequences
  {
    fact = fact * i;
  }

  rearange(numbers, num, num, fact); // begin rearranging by recursion

  return 0;
}
Kaunteya
  • 3,107
  • 1
  • 35
  • 66
Slayter
  • 1,172
  • 1
  • 13
  • 26
  • GDB should tell you where the seg-fault/core dump happened, and how deep the stack is when the crash occurs. What does it say? – abelenky Feb 27 '13 at 21:42
  • the fact variable shows how many iterations are left. When 9 is inputted, there are 188202 iterations remaining when the program crashes. and it says it occurs during the `printf()` statement. – Slayter Feb 27 '13 at 21:45
  • You are avoiding using GDB. Use of a debugger is *essential* in programming. If you have a core file, you should really learn to load it up in GDB and inspect things like the stack depth and the crash location. – abelenky Feb 27 '13 at 22:43
  • @Slayter dont know if you are still there but please try the code i wrote in my "Edit 2": It solves your problem with a recursion-depth equal to the number of items (e.g. 10). – Bernd Elkemann Feb 27 '13 at 23:02
  • I was using GDB to get that information – Slayter Feb 28 '13 at 01:43
  • @Slayter ah you are back. Did you try the solution in my "EDIT 2"? – Bernd Elkemann Feb 28 '13 at 06:20
  • @Slayter have you looked at the EDIT 2 in my solution? It runs perfectly. (There is no need to increase your stack-size.) – Bernd Elkemann Mar 03 '13 at 09:46
  • Thanks but I figured it out a while back. Also I didn't want someone to write it for me, I only needed advice as to where I was going wrong. – Slayter Mar 04 '13 at 04:13

5 Answers5

6

9! (362880) and 10! (3628800) are huge numbers that overflow the call stack when you do as many recursive calls. Because all the local variables and formal parameters have to be stored. You either you have to increase the stack size or convert the recursion into iteration.

On linux, you can do:

ulimit -s unlimited

to set the stack size to unlimited. The default is usually 8MB.

Stephane Rolland
  • 38,876
  • 35
  • 121
  • 169
P.P
  • 117,907
  • 20
  • 175
  • 238
  • And you pass table by value that gives you 11 * 4 bytes for array and another 3*4 for arguments... thast a lot of memory thats left on stack. I think redesign of concept will be far better then resizeing stack. – Marcin Waśniowski Feb 27 '13 at 21:52
  • How could I go about converting it to iteration. This is a school project so I'm not sure increasing the stack size is an option – Slayter Feb 27 '13 at 21:52
  • @Slayter 1. pass pointer or reference, not array 2. i think you can call it in loop, as you dont use result of last pass, so you dont need recurency – Marcin Waśniowski Feb 27 '13 at 21:55
  • recurency is a requirement of this assignment :/ – Slayter Feb 27 '13 at 21:56
  • Will be hard, 10! calls with 4bytes on stack overhead, gives over 13mb just for recursion. Just a hint as it's very late here. Is it allowed to print more then single combination at single call? – Marcin Waśniowski Feb 27 '13 at 22:03
  • If you *must* use recursion, then you should consider increasing the stack size on your platform. [This page](http://msdn.microsoft.com/en-us/library/8cxs58a6.aspx) shows how to increase the stack size in case of windows (MSVS). If you explain, I am sure your lecturer won't have any problem with that :) – P.P Feb 27 '13 at 22:06
  • spoke with my professor and he said the stack size is a possibility. but I also thought of a solution for iteration that may work with recursion. I'll try this and see how it goes. Thanks for the help! – Slayter Feb 27 '13 at 22:08
  • @slayter even if you compute the permutations of 9 entries recursively you only need 9 stackframes. – Bernd Elkemann Feb 27 '13 at 22:17
  • @KingsIndian Yes, i am downvote on this. You assume recursion needs 1 stackframe per permutation not per level. Permutations of 10 elements only needs 10 stackframes. – Bernd Elkemann Feb 27 '13 at 22:37
  • @eznme Which part of my answer says that? – P.P Feb 27 '13 at 22:38
  • @KingsIndian No, I say that. You claim 10! overflows the stack because it needs such deep recursion, but i say it only needs 10 stackframes. – Bernd Elkemann Feb 27 '13 at 22:41
  • I ended up using a iteration through my recursive function by only having it called `n` times then back out and do it again as necessary. I didn't realize how poor my original code was, I was slightly proud of it :/ Also the output I put in the question was actually given by my professor so maybe he was wrong? – Slayter Feb 28 '13 at 21:43
  • @Slayter For n, there are `n!` permutations. So that's what you have in your example. So that's correct. Whether you have to remove the duplicate or not is upto assignment's conditions. – P.P Feb 28 '13 at 22:13
2

Calculating permutations can be done iteratively, but even if you do it recursively there is no need to have a gigantic stack (like answers suggesting to increase your system stack say). In fact you only need a tiny amount of your stack. Consider this:

0 1      <- this needs **2** stackframes 
1 0                and an for-loop of size 2 in each stackframe

0 1 2    <- this needs **3** stackframes 
0 2 1              and an for-loop of size 3 in each stackframe
1 0 2
1 2 0
2 1 0
2 0 1

Permuting 9 elements takes 9 stackframes and a for-loop through 9 elements in each stackframe.

EDIT: I have taken the liberty to add a recursion-counter to your rearrange-function, it now prints like this:

Enter a number less than 10: 4
depth 1      1 2 4 3
depth 2      1 4 2 3
depth 3      4 1 2 3
depth 4      4 1 3 2
depth 5      4 3 1 2
depth 6      3 4 1 2
depth 7      3 4 2 1
depth 8      3 2 4 1
depth 9      2 3 4 1
depth 10      2 3 1 4
depth 11      2 1 3 4
depth 12      1 2 3 4
depth 13      1 2 4 3
depth 14      1 4 2 3
depth 15      4 1 2 3
depth 16      4 1 3 2  which is obviously wrong even if you do it recursively.
depth 17      4 3 1 2
depth 18      3 4 1 2
depth 19      3 4 2 1
depth 20      3 2 4 1
depth 21      2 3 4 1
depth 22      2 3 1 4
depth 23      2 1 3 4
depth 24      1 2 3 4
....

The recursion-leafs should be the only ones which output so the depth should be constant and small (equal to the number you enter).

EDIT 2:

Ok, wrote the code. Try it out:

#include "stdio.h"
void betterRecursion(int depth, int elems, int* temp) {
    if(depth==elems) {
        int j=0;for(;j<elems;++j){
            printf("%i ", temp[j]);
        }
        printf("   (at recursion depth %u)\n", depth);
    } else {
        int i=0;for(;i<elems;++i){
            temp[depth] = i;
            betterRecursion(depth+1, elems, temp);
        }
    }
}
int main() {
    int temp[100];
    betterRecursion(0, 11, temp); // arrange the 11 elements 0...10
    return 0;
}
Bernd Elkemann
  • 23,242
  • 4
  • 37
  • 66
  • @KingsIndian Let me write some code to try it out. If it is correct i will upvote of course. – Bernd Elkemann Feb 27 '13 at 22:44
  • 1
    @KingsIndian eznme is right, if you go for "divide and conquer" witch is main concept of recursion, then you need no more then as many frame stack as num entered by user. If num = 1 then just output {1}, if num is 2 then outup {1 2}, swap and output {2 1}, if it's 3.. then print first val {1}, and recursivly try to print rest of table... and so on... this should work – Marcin Waśniowski Feb 27 '13 at 22:44
  • Ok wrote the code. Works with recursion-depth equal to number of entries. – Bernd Elkemann Feb 27 '13 at 22:50
  • @eznme You are talking about your `betterRecursion`. Do you say OP's code will work with a stack size of *MB or less for a permutation of 10 i.e. 10! ? My answer says it'll break and you say it won't. Did you try with a stack size 8MB or less? This seems too obvious to me to be arguing about it. – P.P Feb 27 '13 at 22:56
  • @KingsIndian Just try my code. Factorials are of no interest here you dont need to store all combinations at once on the stack, just the current combination. – Bernd Elkemann Feb 27 '13 at 22:59
  • @eznme You are not answering my question. You said the call stack size doesn't matter and downvoted saying that. Does OP's code work with a stack size of less than 8MB? – P.P Feb 27 '13 at 23:01
1

I'd make your rearange function iterative - do while added, and recursive call removed:

void rearange(int numbers[11], int index, int num, int fact) {
    int temp;
    do
    {
      temp = numbers[index];
      numbers[index] = numbers[index-1];
      numbers[index-1] = temp;

      int i;
      for (i = 1; i <= num; ++i) // print the current sequence
      {
        printf("%d ", numbers[i]);
      }
      printf("\n");

      fact--;  // decrement how many sequences remain
      index--; // decrement our index in the array

      if (index == 1) // if we're at the beginning of the array
        index = num;    // reset index to end of the array

    } while (fact > 0);
}
chue x
  • 18,573
  • 7
  • 56
  • 70
0

This is not a task for a deep recursion. Try to invent some more stack-friendly algorithm. Following code has rather troubles with speed than with stack size... It's a bit slow e.g. for n=1000 but it works.

#include <stdio.h>

void print_arrangement(int n, int* x)
{
  int i;
  for(i = 0; i < n; i++)
  {
  printf("%s%d", i ? " " : "", x[i]);
  }
  printf("\n");
}

void generate_arrangements(int n, int k, int* x)
{
    int i;
    int j;
    int found;

    if (n == k)
    {
        print_arrangement(n, x);
    }
    else
    {
    for(i = 1; i <= n; i++)
    {
        found = 0;
        for(j = 0; j < k; j++)
        {
            if (x[j] == i)
            {
                found = 1;
            }
        }
        if (!found)
        {
            x[k] = i;
            generate_arrangements(n, k + 1, x);
        }
    }   
    }
}

int main(int argc, char **argv)
{
  int x[50];
  generate_arrangements(50, 0, x);
}
V-X
  • 2,979
  • 18
  • 28
0

Your program is using too many recursions unnecessarily. It is using n! recursions when actually n would be enough.

To use only n recursions, consider this logic for the recursive function:

  • It receives an array nums[] of n unique numbers to arrange
  • The arrangements can have n different first number in them, as there are n different numbers in the array
  • (key step) Loop over the elements of nums[], and in each iteration create a new array but with the current element removed, and recurse into the same function passing this shorter array as parameter
  • As you recurse deeper, the parameter array will be smaller and smaller
  • When there is only one element left, that's the end of the recursion

Using this algorithm, your recursion will not be deeper than n and you will not get segmentation fault. The key is in the key step, where you build a new array of numbers that is always 1 item shorter than the input array.

As a side note, make sure to check the output of your program before you submit, for example run it through | sort | uniq | wc -l to make sure you are getting the correct number of combinations, and check that there are no duplicates with | sort | uniq -d (the output should be empty if no dups).

Spoiler alert: here's my implementation in C++ using a variation of the above algorithm: https://gist.github.com/janosgyerik/5063197

janos
  • 120,954
  • 29
  • 226
  • 236
  • upon running your code I see it gave the same output as mine. Mine was simply in a different order which is not a requirement of the assignment. There are no duplicate lines in my output (the output listed in the question however was my professor's). And I know I don't need that many recursions, that was the reason for the question. Also I should've made it clear in the question that I didn't need someone to rewrite my code since this is a school assignment I was simply looking for some advice. Thanks though. – Slayter Feb 28 '13 at 21:56
  • I have rewritten my code it may be slightly too complex for the problem but with the experience I have in programming it worked for me. Here it is: [link](http://pastebin.com/RmJjiuuc) – Slayter Feb 28 '13 at 22:10