Issues with 2D array peaks finder-CodePudding

I wrote this code with help of a tutorial, the code works fine but it does not output the answers I am supposed to get the answer that it gives me are (2,4) and (3,3) which are 5530 and 5411 when I should be getting (2,1), (2,5), and (4,3) which are 5560, 5821, and 5831. the code should only output those 3 values

the code goes as follows

#include <stdio.h>

#define columns 7
#define rows 6

int arr[rows][columns] = {
   { 5039,  5127,  5238,  5259,  5248,  5310,  5299 },
   { 5150,  5392,  5410,  5401,  5320,  5820,  5321 },
   { 5290,  5560,  5490,  5421,  5530,  5831,  5210 },
   { 5110,  5429,  5430,  5411,  5459,  5630,  5319 },
   { 4920,  5129,  4921,  5821,  4722,  4921,  5129 },
   { 5023,  5129,  4822,  4872,  4794,  4862,  4245 },
};

void find_peaks(int arr[][columns]);

int main() {
    
    find_peaks(arr);

    return 0;
}

void find_peaks(int arr[][columns]) {
    int curr = 0, peak_counter = 0;

    for (int i = 0; i < rows - 1; i  )
        for (int j = 1; j < columns - 1; j  ) {
            curr = arr[i][j];
            for (int k = 0; k <= i   1; k  ) {
                for (int l = j - 1; l <= j   1; l  ) {
                    if (arr[k][l] < curr)
                        peak_counter  ;
                    if (arr[k][l] > curr)
                        curr = arr[i][j];
                }
            }

            if (peak_counter == 8)
                printf("peak at (%ld,%ld)\n", i, j);
            
            peak_counter = 0;
        }
}

I would appreciate some help.

CodePudding user response：

Your code to find local maxima has some issues:

the outer loop should start at i = 1
the third loop should start at k = i - 1
updating curr inside the inner loop seems unwarranted. You could break from the inner loop as soon as a test fails and k != i and l != j, but these extra tests might hurt performance instead of improving it. As coded, the inner loops should compile to efficient branchfree code.
you should use %d for int arguments in printf("peak at (%ld,%ld)\n", i, j);
it is idiomatic to use uppercase for the macros columns and rows.

Here is a modified version:

#include <stdio.h>

#define COLUMNS 7
#define ROWS 6

int array[ROWS][COLUMNS] = {
    { 5039,  5127,  5238,  5259,  5248,  5310,  5299 },
    { 5150,  5392,  5410,  5401,  5320,  5820,  5321 },
    { 5290,  5560,  5490,  5421,  5530,  5831,  5210 },
    { 5110,  5429,  5430,  5411,  5459,  5630,  5319 },
    { 4920,  5129,  4921,  5821,  4722,  4921,  5129 },
    { 5023,  5129,  4822,  4872,  4794,  4862,  4245 },
};

void find_peaks(int arr[][COLUMNS]) {
    for (int i = 1; i < ROWS - 1; i  ) {
        for (int j = 1; j < COLUMNS - 1; j  ) {
            int peak_counter = 0;
            int curr = arr[i][j];
            for (int k = i - 1; k <= i   1; k  ) {
                for (int l = j - 1; l <= j   1; l  ) {
                    if (arr[k][l] < curr)
                        peak_counter  ;
                }
            }
            if (peak_counter == 8)
                printf("peak at (%d,%d): %d\n", i, j, curr);
        }
    }
}

int main() {
    find_peaks(array);
    return 0;
}

Output:

peak at (2,1): 5560
peak at (2,5): 5831
peak at (4,3): 5821

CodePudding user response：

Here is how I would solve the problem.

#include <stdio.h>

#define columns 7
#define rows 6

int arr[rows][columns] = {
   { 5039,  5127,  5238,  5259,  5248,  5310,  5299 },
   { 5150,  5392,  5410,  5401,  5320,  5820,  5321 },
   { 5290,  5560,  5490,  5421,  5530,  5831,  5210 },
   { 5110,  5429,  5430,  5411,  5459,  5630,  5319 },
   { 4920,  5129,  4921,  5821,  4722,  4921,  5129 },
   { 5023,  5129,  4822,  4872,  4794,  4862,  4245 },
};

void find_peaks(int arr[][columns]);

int main() {
    
    find_peaks(arr);

    return 0;
}

void find_peaks(int arr[][columns]) {
    int curr = 0;

    for (int i = 1; i < rows - 1; i  )
        for (int j = 1; j < columns - 1; j  ) {
            curr = arr[i][j];
            //left, right, down, up, upper_left, upper_right, lower_left, lower_right
            if (arr[i][j-1] < curr && arr[i][j 1] < curr && arr[i-1][j] < curr && arr[i 1][j] < curr && arr[i-1][j-1] < curr && arr[i-1][j 1] < curr && arr[i 1][j-1] < curr && arr[i 1][j 1] < curr) {

                printf("peak at (%d,%d)\n", i, j);
            }
        }
}

CodePudding user response：

I can't figure out how your algorithm is intended to work, so I just wrote my own.

#include <stdio.h>

#define COLUMNS 7
#define ROWS 6

int arr[ROWS][COLUMNS] = {
   { 5039,  5127,  5238,  5259,  5248,  5310,  5299 },
   { 5150,  5392,  5410,  5401,  5320,  5820,  5321 },
   { 5290,  5560,  5490,  5421,  5530,  5831,  5210 },
   { 5110,  5429,  5430,  5411,  5459,  5630,  5319 },
   { 4920,  5129,  4921,  5821,  4722,  4921,  5129 },
   { 5023,  5129,  4822,  4872,  4794,  4862,  4245 },
};

void find_peaks() {

  int get (int x, int y) {
    if (x < 0 || x >= ROWS) return -1;
    if (y < 0 || y >= COLUMNS) return -1;
    return arr[y][x];
  };

  for (int y = 0; y < ROWS; y  ) {
    for (int x = 0; x < COLUMNS; x  ) {
      int t = arr[y][x];
      if (t <= get(x   1, y)) continue;
      if (t <= get(x - 1, y)) continue;
      if (t <= get(x, y   1)) continue;
      if (t <= get(x, y - 1)) continue;
      if (t <= get(x   1, y   1)) continue;
      if (t <= get(x - 1, y   1)) continue;
      if (t <= get(x   1, y - 1)) continue;
      if (t <= get(x - 1, y - 1)) continue;
      printf("Peak of %d at (%d, %d)\n", t, x, y);
    };
  };
};

int main() {
  find_peaks();
  return 0;
};

If you don't actually care to find peaks on the edge of the array then you don't need the get() wrapper to check that you're not exceeding the array bounds, but instead you can just make the loops go from 1 to limit-1 instead. You can also delete the second set of four if () continue statements if you don't care about the diagonals, but given what you indicate the correct answers are, it appears that you do.