wrong output for memoization solution of knapsack issue

Question

I need to solve the knapsack problem recursively, memoized and with dynamic programming. Currently I'm stuck at the memoized method (and partially the dynamic programming method).

I adapted the code from what I found elsewhere on the internet. Currently the output is not correct.

The problem involves profit and mass. Each item has a profit and mass associated, there is a MAX_N (umber) of items available and a MAX_CAPACITY for mass. The aim is to have as much "profit" in the knapsack as possible.

Here is an example provided by the exercise:

Example: Given a knapsack of capacity 5, and items with mass[] = {2, 4, 3, 2} and profit profit[] = {45, 40, 25, 15}, the best combination would be item 0 (with mass 2 and profit 45) and item 2 (with mass 3 and with profit 25) for a total profit of 70. No other combination with mass 5 or less has a greater profit.

Here is the complete code:

#include <stdio.h>

#define MAX_N 10
#define MAX_CAPACITY 165

int m[MAX_N+1][MAX_CAPACITY+1];

int max(int x, int y) {
    return x ^ ((x ^ y) & -(x < y));
}

int min(int x, int y) {
    return y ^ ((x ^ y) & -(x < y));
}

int knapsackRecursive(int capacity, int mass[], int profit[], int n) {

    if (n < 0)
        return 0;

    if (mass[n] > capacity)
        return knapsackRecursive(capacity, mass, profit, n-1);

    else
        return max(knapsackRecursive(capacity, mass, profit, n-1), knapsackRecursive(capacity - mass[n], mass, profit, n-1) + profit[n]);

}

int knapsackMemoized(int capacity, int mass[], int profit[], int n) {

    int take = 0;
    int dontTake = 0;

    if (m[n][capacity] != 0)
        return m[n][capacity];

    if (n == 0) {

        if (mass[0] <= capacity) {
            m[n][capacity] = profit[0];
            return profit[0];
        }

        else {
            m[n][capacity] = 0;
            return 0;
        }
    }

    if (mass[n] <= capacity)
        take = profit[n] + knapsackMemoized(capacity-mass[n], mass, profit, n-1);

    dontTake = knapsackMemoized(capacity, mass, profit, n-1);

    m[n][capacity] = max(take, dontTake);

    return m[n][capacity];

}

int knapsackDynamic(int capacity, int mass[], int profit[], int n) {

    // this only works with int m[MAX_N+1][MAX_CAPACITY+1];

    int i;
    int j;

    for (i = 0; i <= n; i++) {

        for (j = 0; j <= capacity; j++) {

            if (i == 0 || j == 0)
                m[i][j] = 0;

            else if (mass[i-1] <= j)
                m[i][j] = max(profit[i-1] + m[i-1][j-mass[i-1]], m[i-1][j]);

            else
                m[i][j] = m[i-1][j];
        }
    }

    return m[n][capacity];

}

void test() {

    // test values
    //int M1[MAX_N] = {2, 4, 3, 2};
    //int P1[MAX_N] = {45, 40, 25, 10};

    int M1[MAX_N] = {6, 3, 2, 4};
    int P1[MAX_N] = {50, 60, 40, 20};

    int M2[MAX_N] = {23, 31, 29, 44, 53, 38, 63, 85, 89, 82};
    int P2[MAX_N] = {92, 57, 49, 68, 60, 43, 67, 84, 87, 72};

    // a)
    printf("Recursion: %d\n",knapsackRecursive(MAX_CAPACITY, M1, P1, MAX_N));
    printf("Recursion: %d\n",knapsackRecursive(MAX_CAPACITY, M2, P2, MAX_N));
    printf("\n");

    // b)
    printf("Memoization: %d\n",knapsackMemoized(MAX_CAPACITY, M1, P1, MAX_N));
    printf("Memoization: %d\n",knapsackMemoized(MAX_CAPACITY, M2, P2, MAX_N));
    printf("\n");

    // c)
    printf("Dynamic Programming: %d\n",knapsackDynamic(MAX_CAPACITY, M1, P1, MAX_N));
    printf("Dynamic Programming: %d\n",knapsackDynamic(MAX_CAPACITY, M2, P2, MAX_N));

}

int main() {
    test();
}

This is the output:

Recursion: 170
Recursion: 309

Memoization: 170
Memoization: 170

Dynamic Programming: 170
Dynamic Programming: 309

Process returned 25 (0x19)   execution time : 0.014 s
Press any key to continue.

As you can see, the recursive- and dynamic programming solutions deliver the correct output. (I checked by running the given example arrays through the functions.) The memoization method does currently not. In fact, it always delivers the same result for both arrays, which I'm honestly really confused by.

Another issue (although nowhere near as big) is that the dynamic programming approach only works with int m[MAX_N+1][MAX_CAPACITY+1]; but the exercise calls for int m[MAX_N][MAX_CAPACITY];. I'm not sure how to change the code to support the latter.

Answer 1

So I know this is a homework question, so I won't give the the correct answer but I can say this:

If a function is returning the same answer the second time it is run, it's usually because something hasn't been zeroed. There is some dirt left from the previous run.

Check all places where you assume that variables, that is created outside of the scope of the function, should be clean.

EDIT: For you second question, the m[ITEM][CAP] is a matrix reflecting the profit at each cap left with each item. The items are a loop that runs over all the items once. 1 to MAX or 0 to (MAX-1), that is an array with MAX number of cells. The cap at each item can be everything from 0 to MAX_CAP inclusive. That range is MAX_CAP+1 long, meaning you need to create int m[MAX_N][MAX_CAPACITY+1]. I don't think it's possible to do it in another way, and your code should already work for this.