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

Feature request: Disable operator * for matrices? #26

Closed
PeterHiber opened this issue Sep 1, 2020 · 2 comments
Closed

Feature request: Disable operator * for matrices? #26

PeterHiber opened this issue Sep 1, 2020 · 2 comments
Assignees

Comments

@PeterHiber
Copy link

Hi and thanks for the lib! It's by far one of the least bloated linear algebra libs we have found, and we have had great success with it so far!

Well, except for one aspect. We have had quite a few bugs where people implicitly assumed that the * operator was matrix multiply (and not element-wise multiply). We have become more used to using mul() now, but it's still a problem that pops up every now and then.

So the feature request we have is to be able to disable operator * for matrices (but not for vectors) so that we don't accidentally do element-wise multiplication instead of matrix multiplication.

Have a nice day!

@sgorsten sgorsten self-assigned this Sep 3, 2020
@sgorsten
Copy link
Owner

sgorsten commented Sep 3, 2020

This is probably the single most frequently raised issue among users of this library, and not without good reason. It turns out my own unit tests contained an erroneous usage of operator * when matrix multiplication was intended. Through some quirk of fate, the specific version of doctest I was using wasn't correctly invoking templated test cases, and the broken test slipped through.

I've decided to deprecate operator * between pairs of matrices, and provide an unambiguous cmul(...) function that can be called by users who actually want the component-wise (Hadamard) product between matrices. operator * will remain supported for all other patterns with its original semantics.

It took a little bespoke machinery to do so, but I was able to split the definition of operator * into two overloads, one for matrix * matrix, and one for everything else, and attached a deprecation attribute to the matrix * matrix overload, so it should raise a warning any time it is used.

Furthermore, if you #define LINALG_FORWARD_COMPATIBLE, either before including linalg.h or globally across your project, it will remove the matrix * matrix overload completely, making that construct a hard compiler error.

Take care, and happy coding!

@sgorsten sgorsten closed this as completed Sep 3, 2020
@PeterHiber
Copy link
Author

This is excellent! Huge thanks for this, it helps us a lot! 🎉👍

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