Sign up ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free.

I wrote a program to generate all possible matrices with some conditions.

It takes argument 'r' and 'n', where 'r' is the number of rows or columns in a matrix, and 'n' is the maximum number that the sum in each row or column could be

The conditions are:

  1. All of the entries in each row and each column are in non-asccending order
  2. The sum in each row and each column should be smaller than or equal to 'n'
  3. The entries in the main diagonal are in non-ascending order( m[i][i] >= m[j][j] for all i>j)

It also has to generate all possibilities that satisfies the conditions.

So, my approach was generating 0th row and column first, and then 1st row and column, and so on, until I reach 'r'th row and column.

I did this by setting left top entry iteratively using for loops, and the calling genWat function with that left top entry, to generate all possible row entries and column entries using Boost library for combination with replacement.(http://photon.poly.edu/~hbr/boost/combination.hpp)

Then it checks the conditions, and if the generated entries fits the condition(pass the test), they are stored in the matrix(m in class Wat), and again, for the next row and column(I called it 'target'. For example, if it's dealing with the 2nd row and column, target is 2) iteratively setting left top entry and recursively calling genWat function with those left top entries.

It continues until it generate rth row and column(target == r), and if rth row and column satisfies the condition, it stores that Wat into the vector of Wat.

It was probably the most complicated program I have written ever, largely because it uses both recursion and iteration, and I found this problem really interesting, but I could not make it work, and had to ask for help.

It compiles, but when I ran the program with some small values of r and n, like 2 and 4, it keeps running forever. For r=1, it throws out_of_bounds exception.

I think recursion is not quitting because the base case for the recursion is not defined well, but I have no idea why the program is not working.

The code is admittedly messy and long and complicated, but please help me figure out how to do this problem.

Thank you.

    #include<iostream>
    #include<vector>
    #include<string>
    #include<stdlib.h>
    #include<algorithm>
    #include"combination.hpp"
    using namespace std;

    class Wat {
     public:
        int r, n;
        vector<vector<int> > m;
        vector<int> sumRow;
        vector<int> sumCol;

        Wat(const int r, const int n)
         : r(r), n(n)
           m(vector<vector<int> > (r, vector<int> (r, 0))),
           sumRow(vector<int> (r, 0)),
           sumCol(vector<int> (r, 0)) { }

        ~Wat() {
            //delete m;
            //delete sumRow;
            //delete sumCol;
        }

        Wat(const Wat& source) {
            r=source.r;
            n=source.n;
            m = source.m;
            sumRow = source.sumRow;
            sumCol = source.sumCol;
        }

        Wat operator=(const Wat& rhs) {
            Wat tmp(rhs);
            std::swap(r, tmp.r);
            std::swap(n, tmp.n);
            std::swap(m, tmp.m);
            std::swap(sumRow, tmp.sumRow);
            std::swap(sumCol, tmp.sumCol);
        }

        void index_assign(int row, int col, int item) {
            (m.at(row)).assign(col, item);
            sumRow[row] += item;
            sumCol[col] += item;
        }
        void row_assign(int row, int startColIdx, vector<int> items) {
            for(int i = 0; i < items.size(); ++i) {
                index_assign(row, startColIdx + i, items.at(i));
            }
        }
        void col_assign(int startRowIdx, int col, vector<int> items) {
            for(int i = 0; i < items.size(); ++i) {
                index_assign(startRowIdx + i, col, items.at(i));
            }
        }
        bool checkSumForRow(int target, const vector<int>& gen_row) const {
            bool ret = true;
            int testedSum = sumRow[target];
            for (int i=0; i<gen_row.size(); ++i) {
                if(sumCol[target+1+i] + gen_row[i] > n) {
                    ret = false;
                }
                testedSum += gen_row[i];
            }
            if (testedSum > n) {
                ret = false;
            }
            return ret;
        }
        bool checkSumForCol(int target, const vector<int>& gen_col) const {
            bool ret = true;
            int testedSum = sumCol[target];
            for (int i=0; i<gen_col.size(); ++i) {
                if(sumRow[target+1+i] + gen_col[i] > n) {
                    ret = false;
                }
                testedSum += gen_col[i];
            }
            if (testedSum > n) {
                ret = false;
            }
            return ret;
        }
    };

    bool isNonAscending (const vector<int>& v);
    void genWat(const Wat& w, int target, int r, int n, vector<Wat>& vw);

    int main(int argc, char** argv) {
        if(argc != 3) {
            cerr << "arguments: r and n" << endl;
            return 0;
        }
        else {
            vector<Wat> v;
            int r = atoi(argv[1]);
            int n = atoi(argv[2]);
            Wat waat(r, n); //starts from empty Wat, make a copy of previous Wat if needed, and add to v when complete
            for (int i = 0; i < n; ++i) {
                waat.index_assign(0, 0, i);
                genWat(waat, 0, r, n, v);
            }

            return 1;
        }
    }

    void genWat(const Wat& w, int target, int r, int n, vector<Wat>& vw) {
        if(target == r) {
            //compare the entries on each side beside diagonal first
            vw.push_back(w);
        }
        else if(target == r+1) {//might be the base case?? {
            return;
        }

        else {
            std::vector<int> gen_row(r-1, 0);

            do {
                if (isNonAscending(gen_row)) {
                    //need to define assignment operator, but actually to make it efficient, no need to make a copy here(just make a copy of sumRow and sumCol, and check the sum)
                    if (w.checkSumForRow(target, gen_row)) {
                        std::vector<int> gen_col(r-1, 0);
                        do {
                            if(isNonAscending(gen_col)) {
                                if(w.checkSumForCol(target, gen_col)) {
                                    Wat waaat = w;
                                    waaat.row_assign(target, target+1, gen_row);
                                    waaat.col_assign(target+1, target, gen_col);
                                    int leftTopBound = min((waaat.m)[target][target], waaat.n - max(waaat.sumRow[target+1], waaat.sumCol[target+1]));
                                    for (int i = 0; i < leftTopBound; ++i) {
                                        waaat.index_assign(target+1, target+1, i);
                                        genWat(waaat, target+1, r, n, vw);
                                    }
                                }
                            }
                        } while (boost::next_mapping(gen_col.begin(), gen_col.end(), 0, w.m[target][target]));
                    }
                }
            } while (boost::next_mapping(gen_row.begin(), gen_row.end(), 0, w.m[target][target]));
        }
    }

    bool isNonAscending (const vector<int>& v) {
        for(int i=0; i < v.size()-1; ++i) {
            if(v.at(i) < v.at(i+1)) {
                return false;
            }
        }
        return true;
    }
share|improve this question

closed as too localized by Luchian Grigore, Tadeusz Kopec, talonmies, A.H., greedybuddha Jun 12 '13 at 17:55

This question is unlikely to help any future visitors; it is only relevant to a small geographic area, a specific moment in time, or an extraordinarily narrow situation that is not generally applicable to the worldwide audience of the internet. For help making this question more broadly applicable, visit the help center.If this question can be reworded to fit the rules in the help center, please edit the question.

9  
Does this even compile? delete m; delete sumRow; delete sumCol;? – Luchian Grigore Jun 12 '13 at 14:54
    
I suggest single-stepping or emit some output at points so you can see where it is still on track and where derailed. Also when it runs "endlessly" you can break in and start stepping in debugger and look at state – Balog Pal Jun 12 '13 at 14:55
    
and what is the 3 delete in the dtor? – Balog Pal Jun 12 '13 at 14:56
    
You're not likely to get help on this one because it appears you're using a custom boost extension? The Combination library? As such, other people won't be able to compile this, not to mention the fact that you appear to be calling delete on non-pointer/array types. – aardvarkk Jun 12 '13 at 14:58
1  
You need to trim it down to just the problematic recursion. I'm not going to sift through all the other bad code, and don't expect anyone else to. You say it yourself, that there is no well-defined base case; and without that, recursion will not stop. If it were recursing infinitely you should crash with a segmentation fault. Running forever without crashing is more of an infinite loop. – John Jun 12 '13 at 15:01

2 Answers 2

up vote 2 down vote accepted

First, a note: There are many problems with your code. It's recommended that you post questions in such a way that they're easy for people to help with. Your code does not compile as posted and requires the use of a third-party header file. Both your destructor and constructor have errors. Regardless, in relation to the problem of the case of r and n both being equal to 1, the following line is the problem:

std::vector<int> gen_row(r-1, 0);

This is called inside of the function genWat. With r equal to 1, you are trying to make a vector of size 0. In and of itself, this isn't necessarily a problem. But then you call isNonAscending and try to check the 0th index -- but the 0th index doesn't exist because the vector is of size 0!

I would focus on fixing the crash for your simplest case (r = 1 and n = 1) before trying to fix your infinite loop issues.

EDIT: For anybody else who's interested in taking a look, the following code compiles for me on VS2010, and I believe it follows the spirit of the original code. You obviously also need to download combination.hpp from the link given in the question.

#include <iostream>
#include <vector>
#include <string>
#include <stdlib.h>
#include <algorithm>
#include "combination.hpp"
using namespace std;

class Wat {
public:
  int r, n;
  vector< vector<int> > m;
  vector<int> sumRow;
  vector<int> sumCol;

  Wat(const int r, const int n)
    : r(r), n(n), m(r), sumRow(r), sumCol(r) { }

  Wat(const Wat& source) {
    r=source.r;
    n=source.n;
    m = source.m;
    sumRow = source.sumRow;
    sumCol = source.sumCol;
  }

  Wat operator=(const Wat& rhs) {
    Wat tmp(rhs);
    std::swap(r, tmp.r);
    std::swap(n, tmp.n);
    std::swap(m, tmp.m);
    std::swap(sumRow, tmp.sumRow);
    std::swap(sumCol, tmp.sumCol);
  }

  void index_assign(int row, int col, int item) {
    (m.at(row)).assign(col, item);
    sumRow[row] += item;
    sumCol[col] += item;
  }
  void row_assign(int row, int startColIdx, vector<int> items) {
    for(int i = 0; i < items.size(); ++i) {
      index_assign(row, startColIdx + i, items.at(i));
    }
  }
  void col_assign(int startRowIdx, int col, vector<int> items) {
    for(int i = 0; i < items.size(); ++i) {
      index_assign(startRowIdx + i, col, items.at(i));
    }
  }
  bool checkSumForRow(int target, const vector<int>& gen_row) const {
    bool ret = true;
    int testedSum = sumRow[target];
    for (int i=0; i<gen_row.size(); ++i) {
      if(sumCol[target+1+i] + gen_row[i] > n) {
        ret = false;
      }
      testedSum += gen_row[i];
    }
    if (testedSum > n) {
      ret = false;
    }
    return ret;
  }
  bool checkSumForCol(int target, const vector<int>& gen_col) const {
    bool ret = true;
    int testedSum = sumCol[target];
    for (int i=0; i<gen_col.size(); ++i) {
      if(sumRow[target+1+i] + gen_col[i] > n) {
        ret = false;
      }
      testedSum += gen_col[i];
    }
    if (testedSum > n) {
      ret = false;
    }
    return ret;
  }
};

bool isNonAscending (const vector<int>& v);
void genWat(const Wat& w, int target, int r, int n, vector<Wat>& vw);

int main(int argc, char** argv) {
  if(argc != 3) {
    cerr << "arguments: r and n" << endl;
    return 0;
  }
  else {
    vector<Wat> v;
    int r = atoi(argv[1]);
    int n = atoi(argv[2]);
    Wat waat(r, n); //starts from empty Wat, make a copy of previous Wat if needed, and add to v when complete
    for (int i = 0; i < n; ++i) {
      waat.index_assign(0, 0, i);
      genWat(waat, 0, r, n, v);
    }

    return 1;
  }
}

void genWat(const Wat& w, int target, int r, int n, vector<Wat>& vw) {
  if(target == r) {
    //compare the entries on each side beside diagonal first
    vw.push_back(w);
  }
  else if(target == r+1) {//might be the base case?? {
    return;
  }

  else {
    std::vector<int> gen_row(r-1, 0);

    do {
      if (isNonAscending(gen_row)) {
        //need to define assignment operator, but actually to make it efficient, no need to make a copy here(just make a copy of sumRow and sumCol, and check the sum)
        if (w.checkSumForRow(target, gen_row)) {
          std::vector<int> gen_col(r-1, 0);
          do {
            if(isNonAscending(gen_col)) {
              if(w.checkSumForCol(target, gen_col)) {
                Wat waaat = w;
                waaat.row_assign(target, target+1, gen_row);
                waaat.col_assign(target+1, target, gen_col);
                int leftTopBound = min((waaat.m)[target][target], waaat.n - max(waaat.sumRow[target+1], waaat.sumCol[target+1]));
                for (int i = 0; i < leftTopBound; ++i) {
                  waaat.index_assign(target+1, target+1, i);
                  genWat(waaat, target+1, r, n, vw);
                }
              }
            }
          } while (boost::next_mapping(gen_col.begin(), gen_col.end(), 0, w.m[target][target]));
        }
      }
    } while (boost::next_mapping(gen_row.begin(), gen_row.end(), 0, w.m[target][target]));
  }
}

bool isNonAscending (const vector<int>& v) {
  for(int i=0; i < v.size()-1; ++i) {
    if(v.at(i) < v.at(i+1)) {
      return false;
    }
  }
  return true;
}
share|improve this answer

It seems your combination.hpp that you provided doesn't like when your first_value and last_value are the same. You're telling it to generate all the combinations from 0 to 0 and it's never stopping. I changed line 269 from:

     if (++(*(--last)) != last_value)

to

     if (++(*(--last)) < last_value)

and the code no longer runs infinitely long.

THIS IS NOT A PROPER FIX! I have no idea if combinations are still correctly being generated by next_mapping and decided to leave it to you from here.

HTH

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.