Any interests in element wise operations on vector/matrix with F#? #450
Conversation
…hose defined as operators in F#) in Matrix and Vector and also added implementations for all the subtypes
…ix and Vector generic types, so that these can be called from F# as if they are normal mathematical operators
…s it to return an integer and is different from the pointwise definition.
… Exp() respectively so that they work with F# mathematical operator syntax
…o overwrite other people's work
| @@ -459,6 +459,75 @@ protected override void DoPointwiseLog(Matrix<Complex> result) | |||
| Map(Complex.Log, result, Zeros.Include); | |||
| } | |||
|
|
|||
| protected override void DoPointwiseAbs(Matrix<Complex> result) | |||
| { | |||
| Map(x => (Complex)Complex.Abs(x), result, Zeros.Include); | |||
There was a problem hiding this comment.
For all operations where f(0) = 0 we can allow Map to skip zeros by specifying Zeros.AllowSkip. This is relevant on sparse matrices, where all the zero entries can be skipped this way.
There was a problem hiding this comment.
Ah, got it, thanks! Will fix it for abs, acos, asin, atan, ceiling, floor, round, sin, sinh, sqrt, tan and tanh
There was a problem hiding this comment.
Sorry, not acos...
| f(result); | ||
| } | ||
|
|
||
| public Matrix<T> PointwiseBinary(Action<Matrix<T>, Matrix<T>> f, Matrix<T> other) |
There was a problem hiding this comment.
Nice catch, thanks! Will fix.
| throw new ArgumentException(Resources.ArgumentVectorsSameLength, "y"); | ||
| } | ||
|
|
||
| var result = Build.SameAs(this); |
There was a problem hiding this comment.
This should consider y as well I think, i.e. Build.SameAs(this, y). Also it seems the other routines use other instead of y as parameter name - although that is not really consistent within the existing codebase either.
| public void PointwiseExp(Matrix<T> result) | ||
| /// <param name="f"></param> | ||
| /// <returns></returns> | ||
| protected void PointwiseUnary(Action<Matrix<T>> f, Matrix<T> result) |
There was a problem hiding this comment.
I wonder whether we actually need these helpers, since we already do range checking within the Map function. Or we could use the unchecked Map implementations. Seems I need to spend some thoughts on the architecture here - but that should not block this PR.
There was a problem hiding this comment.
These are really because I saw the checks done in all the other pointwise functions before this pull request. Since I am adding some 20 more of them, copy and pasting the checks is something I really want to avoid. True, I did not try to refactor the existing ones though. Yeah, please have a think and we can decide what to do here.
There was a problem hiding this comment.
Thanks. It's ok for now, but I'll think about it.
| /// | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| public Matrix<T> PointwiseAbs() |
There was a problem hiding this comment.
We'll need to have proper documentation on all public members in the end.
There was a problem hiding this comment.
Being lazy here, will fix, thanks!
There was a problem hiding this comment.
Thanks. Yes, It's quite tedious; sorry for all the duplication (this would be less an issue with code gen, but we moved away from that a long time ago for good reasons).
| @@ -305,5 +305,90 @@ public static Matrix<T> op_DotHat(Matrix<T> matrix, T exponent) | |||
| { | |||
| return matrix.PointwisePower(exponent); | |||
| } | |||
|
|
|||
| public static Matrix<T> Log(Matrix<T> x) | |||
There was a problem hiding this comment.
These will also need proper documentation.
There was a problem hiding this comment.
Yup, will fix shortly, thanks!
| public void PointwiseExp(Vector<T> result) | ||
| /// <param name="f"></param> | ||
| /// <param name="result"></param> | ||
| protected void PointwiseUnary(Action<Vector<T>> f, Vector<T> result) |
There was a problem hiding this comment.
Please avoid adding empty documentation skeletons, that makes it hard to spot whether we're still lacking some proper documentation.
There was a problem hiding this comment.
True, will fix and avoid going forward.
…he AllowSkip option in Map so that its more efficient for sparse matrices.
… consider both matrices when replicating so that a dense or sparse matrix is created accordingly
|
Merged, thanks a lot! I hope you do not mind that I let GitHub squash the commits together. |
|
Thank you! Don't mind at all, would have been what I wanted to do anyways instead of having so many incremental commits. Glad to be able to contribute! |
Discussion thread is here: http://discuss.mathdotnet.com/t/any-interests-in-element-wise-operations-on-vector-matrix-with-f/367