blasfeo icon indicating copy to clipboard operation
blasfeo copied to clipboard

const correctness

Open roversch opened this issue 6 years ago • 2 comments

Have you ever considered making the input vectors/matrices to the BLASFEO routines const? It seems standard with other BLAS implementations, e.g. Netlib CBLAS does this: http://www.netlib.org/blas/cblas.h

If you change to for example

void blasfeo_daxpy(int kmax, const double alpha, const struct blasfeo_dvec *sx, int xi, const struct blasfeo_dvec *sy, int yi, struct blasfeo_dvec *sz, int zi);

it would have the following advantages:

  • it communicates the intent of your code more clearly (i.e. BLASFEO is not going to change anything in the struct pointed to)
  • const correctness (https://www.cprogramming.com/tutorial/const_correctness.html)
  • don't need casts of the type (blasfeo_dvec *)... or const_cast<blasfeo_dvec *>(...) in wrapper code

roversch avatar Jan 17 '19 16:01 roversch

Thanks for the suggestion. In general, I am against adding the const qualifier to the interfaces, for these reasons:

  • in the BLAS API, there are not, since this was coded 30 years ago or so in fortran
  • in the BLASFEO API, there is the possibility to alias arguments. For example dgemm_nn is defined as D <- alpha*A*B + beta*C and C and D can alias (i.e. be the same matrix), so there is not a clear distinction between inputs (A, B, C) and outputs (D). In such cases, in my opinion having no const qualifier at all is better than having something half-way (only in A and B) or 'wrong' (as it would be to add it to C in case of alias with D)

giaf avatar Jan 23 '19 08:01 giaf

Not sure what you mean with the first point, Fortran doesn't have a notion of const and the C API definitely has const (see link in original post).

About the second point: having const in that case is a feature, not an error. const only means that you can not (and will not) alter the argument with that name. So even though C would be pointer to const and D would be a regular pointer, they can still point to the same object underneath. It provides a nice check that you are not changing the array through C but only through D. I did not think about this example before, but it is an additional argument for putting const everywhere.

roversch avatar Jan 23 '19 10:01 roversch