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

Remove type piracy #24

Open
KristofferC opened this issue Jul 13, 2020 · 7 comments
Open

Remove type piracy #24

KristofferC opened this issue Jul 13, 2020 · 7 comments

Comments

@KristofferC
Copy link
Member

The matrix multiplication here should likely be implemented with an MKLSparse.matmul or something instead of type piracying the Base matrix multiplication with more specific versions.

@pablosanjose
Copy link
Member

pablosanjose commented Jul 13, 2020

There is something to be said in favor of the current type piracy approach. If a package uses the LinearAlgebra mul! API, and a user wants to make use of MKL for such package, it is enough that the user does using MKLSparse. If we remove this type piracy, that becomes impossible, and the package needs to provide a mechanism to switch to MKL.

@KristofferC
Copy link
Member Author

Yeah, that's true. An alternative would be for the sparse matrix multiplication API to let you set a "backend" that does the computation. Basically, separate the concept of sparse matrix multiplication from the implementation that currently is in Base.

@pablosanjose
Copy link
Member

pablosanjose commented Jul 14, 2020

But even in that case, the package would still need to expose the mul! backend as part of its API in some way. In fact, I see this instance of type piracy as the one situation where type piracy is desirable. One usually warns against type piracy because it can change behavior without warning through inter-package interactions. In this case we actually want the "without warning" part. The behavior, however, is not changed, only the performance (by a substantial amount if using multithreading). The idea of MKLSparse is precisely this, hijack the Julia internals to make sparse linalg faster without changing anything else. And I see nothing wrong about that in this case...

@KristofferC
Copy link
Member Author

KristofferC commented Jul 14, 2020

In fact, I see this instance of type piracy as the one situation where type piracy is desirable

Well, what happens if there is another package that provides sparse matrix multiplication using some other backend? And what happens when both those packages happen to be loaded?

@pablosanjose
Copy link
Member

pablosanjose commented Jul 14, 2020

If both packages implement the same behaviour, the result is the same, only performance changes. But I do get your point. The idiomatic Julian approach would be to have the user wrap whatever input in a new type owned by MKLSparse, and have the MKLSparse mul! dispatch on that type. Of course the package receiving the input in question would need to be fully generic, etc. So what should be done here? I fear that MKLSparse would become much less useful by making it more correct...

@jishnub
Copy link

jishnub commented Feb 10, 2023

Perhaps there could be a macro? E.g. A * b uses the SparseArrays methods, whereas @mkls A * b uses the ones defined in this package. This way, toggling between the two would be straightforward.

@KristofferC
Copy link
Member Author

I don't see how that would be straight forward if the A * b is e.g. in some other package.

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

3 participants