0

I'm dealing with an algorithm homework which need to be executable on linux(I'm using wsl), and this homework need to store the solution and export to the output file.

To store the solution, I need to open an 2D array of "vectors storing pairs", and at the end, I need to sort the pairs by its first component in the ascending order.

I know that for the first requirement, the running time would be O(N^2)(This is also the standard answer for this dynamic programming problem), and for the second one, I've google C++ STL, and it says that the algorithm used by c++ only needs O(NlogN).

(1)It return 'killed' if I use the new to declare 2D array of vector, for the cases that N=10000, but it works fine if N=1000.

[Edited] (2) I check the comment, and they suggest that I should write the code using vector to store vector instead of new. However, when I change to using vectors storing vectors, now the program cannot run, keep throwing segmentation fault.

I don't know where is happening. Can anybody help?

problem description: https://drive.google.com/file/d/1m8ISIGlVGXH3oeyechLbBA1QQVSmsw-q/view?usp=sharing

file: https://drive.google.com/file/d/1Ci8MXUsX65oVOxKCD1u3YcWiXsKNYToc/view?usp=sharing

Note:

  1. The .o files are alredy make, finish editing, you need to 'make', and run ./bin/mps [inputfile] [outputfile]
  2. I've modified some code, but it can only run with case N=12, 1000; not for larger N. chord.h:
#include <vector>
#include <utility>
#include <algorithm>
using namespace std;

class Chord {
    public:
        Chord(int);
        ~Chord();
        void setEndPoint(int, int);
        int getMaximumChords();
        void print();
        int size;
        int* data;  //stores the endpoints of the chord
       // vector<pair<int,int>>** sol; 
       //I recently use this(1), works for N=1000, but killed if N larger
        vector< vector< vector<pair<int, int>> >>sol;
       //suggest to change to this(2), not working even for N=12, 
       //return segmentation fault
        int getEndPoint(int);
};

chord.cpp:

#include "chord.h"
#include <iostream>

Chord::Chord(int tmp){ //initialize all elements 0
    size = tmp;
    data = new int [size];
};

Chord::~Chord(){
    delete[] data;
}
void Chord::setEndPoint(int a, int b){
    data[a] = b;
    data[b] = a;
    return;
}

void Chord::print(){
    for(int i=0;i<size; i++){
        cout << data[i] << endl;
    }
    return;
}

int Chord::getEndPoint(int a){
    return data[a];
}

int Chord::getMaximumChords(){
    for(int j=1; j<size; j++){
        for(int i=j-1; i>=0; i--){
            int k = getEndPoint(j);
            if(k==i){ //case 1: ij is the chord
                sol[i][j] = sol[i+1][j-1]; //make a copy
                sol[i][j].reserve(1);
                sol[i][j].push_back(make_pair(i,j));
            }else if(k<i || k>j){ //case 2: k outside interval[i,j]
                sol[i][j] = sol[i][j-1];
            }else{ //case 3: k inside interval[i,j]
                if (sol[i][j-1].size() > sol[i][k-1].size() + sol[k+1][j-1].size() + 1){
                    sol[i][j] = sol[i][j-1];
                }else{
                    sol[i][j] = sol[i][k-1];
                    sol[i][j].reserve(sol[k+1][j-1].size()+1);
                    sol[i][j].push_back(make_pair(k,j));
                    sol[i][j].insert(sol[i][j].end(),sol[k+1][j-1].begin(), sol[k+1][j-1].end()); 
                }
            }
        }
    }
    sort(sol[0][size-1].begin(), sol[0][size-1].end());
    return sol[0][size-1].size();
}

main.cpp

#include <iostream>
#include <fstream>
#include <stdio.h>
#include <stdlib.h>
#include "chord.h"

using namespace std;
int main(int argc, char* argv[]){
    if (argc < 3){
        printf("Please enter output file name!");
        return 0;
    }
    //import input
    fstream fi(argv[1]);
    fstream fo;
    fo.open(argv[2], ios::out);
    int N=0, a=0, b=0;
    char buffer[200];
    fi >> N;
    Chord chord(N);
    while(fi>> a >>b){
        chord.setEndPoint(a,b);
    }
    //chord.print();
    int ans= chord.getMaximumChords();

    //export output
    fo << ans <<endl;
    for(int i=0; i<chord.sol[0][chord.size-1].size(); i++){
        fo << chord.sol[0][chord.size-1][i].first << " " << chord.sol[0][chord.size-1][i].second << endl;
    }
    fi.close();
    fo.close();
    return 0;
}
jerry
  • 37
  • 5
  • Maybe you ran out of memory. You can check the syslog to verify. – eerorika Nov 14 '21 at 01:40
  • 5
    `new`ing `vector`s is almost always the wrong thing to do. I don't think it's what your problem is, but can I talk you into a `vector` of `vector`s? – user4581301 Nov 14 '21 at 01:42
  • As you seem to understand `std::vector`, consider that every `new`/`delete` in this code could be replaced with a `std::vector`, and you would no longer be dangerously violating the [Rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – Drew Dormann Nov 14 '21 at 02:04
  • @user4581301 I change the new into an 2D vector, but now it just says segmenataion fault(? – jerry Nov 14 '21 at 03:00
  • All it would take is one call to `setEndPoint()` where either argument's value is not in the range `[0, size-1]` to corrupt memory. I suggest putting `assert()` calls (or similar) at the top of that method to verify that the arguments are valid, before writing into the `data` array using them. – Jeremy Friesner Nov 14 '21 at 03:35
  • @jerry I see that your program takes input file with some data. Can you provide for download all necessary files to reproduce your program's fault on our side? So that we can see identical result as you see on our computer. – Arty Nov 14 '21 at 03:40
  • @arty I've upload the files, thanks for your help!!! – jerry Nov 14 '21 at 04:06
  • @JeremyFriesner the input file first line N is the largest, and any line after it is smaller, it didn't generate problem for the lower case. – jerry Nov 14 '21 at 04:10
  • @jerry Just posted [my solution](https://stackoverflow.com/a/69960290/941531) to your question, at least after my change your algorithm doesn't crash on `10000` input on my Windows laptop. – Arty Nov 14 '21 at 04:33
  • That's a lot of description to go through to find out your program crashes. Related, that's a lot of code to read through to look for something that's off. A good [mre] usually isolates the problem in such a way that we don't need to know what the goal of your program is -- we can focus on just the functionality that causes the crash. – JaMiT Nov 14 '21 at 04:35

1 Answers1

1

By default, std::vector is constructed with 0 size, and I see that you don't ever resize the vector, but you access its elements by index [i][j]. You have to resize first two (or maybe three) dimensions of 3-dimensional vector sol to fit necessary size, do following resize inside constructor:

Chord::Chord(int tmp){ //initialize all elements 0
    size = tmp;
    data = new int [size];
    sol.resize(size, vector< vector<pair<int, int>> >(size));
};

After this resize change in constructor your program doesn't crash on 10000 input, at least on my Windows laptop.

Also maybe you need to resize two dimensions to bigger than size, you should know better. Also 3rd dimension might be needed to resize too, if you need this by algorithm, up to you. If you need to resize 3rd dimension, then do following (but if I understand your algorithm correctly you don't need this change, resizing 3rd dimension, you need it to be of size 0):

sol.resize(size1, vector< vector<pair<int, int>> >(size2, vector<pair<int, int>>(size3)));

(here size1/size2/size3 are three sizes of three dimensions, so that your vector gets 3-dimensional shape (size1, size2, size3), decide what these 3 sizes should be at your algorithm start, I think they should be (size, size, 0))

Arty
  • 14,883
  • 6
  • 36
  • 69
  • Can I ask if this solution is based on declaring the sol by `vector< vector< vector> >>sol` or `vector>** sol`? – jerry Nov 14 '21 at 05:02
  • @jerry My answer above applies to your current code (from question), that is when you declared `vector< vector< vector> >> sol;`. It is better to never use pointers anywhere if possible. Modern C++ has a large `std` library that can solve almost any task without using any pointers at all. – Arty Nov 14 '21 at 05:09
  • Thanks for your help! Now using vector of vectors won't have segmentation error, however it goes back as the first case (When N=10000, killed), as I set (size,size,0). Is it possible that this is due to my wsl don't have enough storage? – jerry Nov 14 '21 at 05:20
  • @jerry On my laptop program was killed due to using too much memory for input `10000`. I have `4 GB` and program has used all of it and was killed afterwards. So either you have to have a bigger-memory PC or to invent a better algorithm. – Arty Nov 14 '21 at 05:22