Segmentation Fault in BPR from python Interface about fastfm

[felixmaximilian]

A new insight:
In ffm_sgc.c, when p_p or p_n "overflows" (running out of the reasonable range), Ai[p_p] is 0, which is why it'll always be smaller then Ai[p_n] which is why it'll be increased again in the if statement, while the reason that another iteration over the while loop has to be done is in fact p_n (because p_n < Ap[sample_row_n + 1] evaluates to True ).
ffm_sgc.c:

int p_n = Ap[sample_row_n];
int p_p = Ap[sample_row_p];
while (p_n < Ap[sample_row_n + 1] || p_p < Ap[sample_row_p + 1]) {
  double grad = 0;
  int i_to_update = Ai[p_p] <= Ai[p_n] ? Ai[p_p] : Ai[p_n];
  double theta_w = coef->w->data[i_to_update];
  // incrementing the smaller index or both if equal
  if (Ai[p_p] == i_to_update) {
    grad = Ax[p_p];
    p_p++;
  }
  if (Ai[p_n] == i_to_update) {
    grad -= Ax[p_n];
    p_n++;
  }

The above PR fixes this issue. Feel free to make it more beautiful :-)

[ibayer]

Regarding the difference between the python and C code, 3) in your first comment.
The C code expect the data in row major order while the python code expects column major (csc_matrix). I have to check in detail if I'm not overlooking something. I mean, a bug like this should really trigger some of the tests...

[ibayer]

@ibayer the choice of column / row major matrices is correct but was hard to understand (PR).

[felixmaximilian]

The mentioned problem is fixed by @ibayer 's PR #14. Thanks for your help!

[ibayer]

@felixmaximilian I have closed your PR for fastFM-core . I have just know updated #63 fastFM-core in fastFM so that the BPR should now work from the Python interface too. Just open a new issue if you run into any other problems. And thanks again for tracing the problem back to it's roots.