-
Notifications
You must be signed in to change notification settings - Fork 692
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
single precision support added for LeastSquares calculation #857
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dineshdharme thanks for submitting this!
So, typically in Breeze we aim for compile time errors rather than runtime errors. Here, we would get a runtime error if we tried to use it for Complex or something. so rather than the match, it would be better if there were two implementations of each of matrixVector
, matrixVectorSpecifiedWork
and matrixVectorSpecifiedWork
Thank you, I will update the PR tomorrow and let you know. |
Hi David, I made changes as you requested. Please take a look and let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost there, thank you
|
||
def apply(x: DenseVector[T]): T = ev match { | ||
|
||
case _: NumericType[Float] => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this method something like
def apply(x: DenseVector[T])(implicit can_dot: OpMulInner.Impl2[DenseVector[T], DenseVector[T], T]) = {
}
and then you can remove all of the switches and exception stuff.
case _ => throw new UnsupportedOperationException("Unsupported numeric type. Only Float and Double are supported") | ||
|
||
} | ||
def apply(X: DenseMatrix[T]): DenseVector[T] = ev match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly
let's make this method something like
def apply(x: DenseVector[T])(implicit can_dot: OpMulMatrix.Impl2[DenseMatrix[T], DenseVector[T], T]) = {
}
and then you can remove all of the switches and exception stuff.
I made the changes you requested. The only difference is for first apply I had to move the Please review. thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Hello David Hall,
This PR adds support for doing Least Squares calculation for Float datatype.
Please review this.
This code should allow you to test the Float datatype.