Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

const correctness #88

Open
roversch opened this issue Jan 17, 2019 · 2 comments
Open

const correctness #88

roversch opened this issue Jan 17, 2019 · 2 comments

Comments

@roversch
Copy link
Contributor

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
@giaf
Copy link
Owner

giaf commented Jan 23, 2019

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)

@roversch
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants