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

times() is not overloaded for Mat x Vec #4

Open
Redrield opened this issue May 25, 2019 · 4 comments
Open

times() is not overloaded for Mat x Vec #4

Redrield opened this issue May 25, 2019 · 4 comments

Comments

@Redrield
Copy link

In the current state of the library, it is impossible to multiply a m x n matrix with an n-dimensional vector. I've gotten around this by adding functions to convert between the types, however this is a construct I think should be supported by the library

I've glanced through the source, and it seems as though the inheritance relationship between these types is backwards. Mat is a subclass of Vec<Vec<E, Cols>, Rows>, however it seems as though Mat should be a top level type, while Vec is the special case where Cols=1.

@breandan
Copy link
Owner

In the current state of the library, it is impossible to multiply a m x n matrix with an n-dimensional vector. I've gotten around this by adding functions to convert between the types, however this is a construct I think should be supported by the library

Yes, this is a good point and it would be nice to support. Currently the only way to perform operations on two tensors of varying rank is by representing the lower rank tensor as an instance of the higher rank with extra dimensions of 1. To support vector-matrix multiplication we can overload the * operator for matrix-vector and vector-matrix multiplication to implicitly represent vectors as Nx1 matrices.

I've glanced through the source, and it seems as though the inheritance relationship between these types is backwards. Mat is a subclass of Vec<Vec<E, Cols>, Rows>, however it seems as though Mat should be a top level type, while Vec is the special case where Cols=1.

This is also a good point. Earlier, we played with both Vec <: Mat and Mat <: Vec, but settled on the latter. After reconsidering our current implementation I suspect you are right - it may be easier to express lower rank tensors as subtypes of higher rank ones. Practically, we need to bound the maximum tensor rank anyway, so I think it could make sense to only support e.g. rank 2 or lower.

If you feel like submitting a PR, I'll be happy to review it. Appreciate the suggestions!

@Redrield
Copy link
Author

Redrield commented May 26, 2019

I've actually taken the dimension stuff and combined it with EJML backed matrices in https://github.com/FRCTeam4069/Keigen, simply because I wanted something thrown together that I could try this with. In that case, I simply made the vector type a typealias:

typealias Vector<D> = Matrix<D, `1`>

That code would take some adapting because it's using EJML instead of rolling a matrix backend from scratch but it could certainly be adapted for use in this project

@breandan
Copy link
Owner

That's a neat solution, thanks for the tip! I'd like to use a linear algebra backend such as ND4J, Commons Math or EJML, but haven't gotten around to it yet. Another library you might want to check out is KMath.

@breandan
Copy link
Owner

After giving this some more thought, I believe either representation could make sense, e.g. representing a Vector as a subtype of Matrix with Cols=1 or Matrix as a subtype of Vector where the elements are of type Vector. I'm still not sure what the right solution is, but for the time being, neither is a subtype of the other, as encoding shape over multiple levels of nested generics is problematic due to issues around erasure and the semantics of type inference in Kotlin. Both sit in their own type hierarchy and we provide operator overloads (e.g. matrix-vector products) where applicable. See #5 for further discussion.

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