-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
ndslice.algorithm #4652
ndslice.algorithm #4652
Conversation
ping @John-Colvin @wilzbach: multidimensional map is here :-) |
@@ -2039,7 +2030,8 @@ struct Slice(size_t _N, _Range) | |||
} | |||
|
|||
static if (doUnittest) | |||
pure nothrow unittest | |||
//pure nothrow |
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.
What prevents this test from being pure
and nothrow
?
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, I forgot to uncomment :-)
@@ -2934,7 +2941,7 @@ unittest | |||
|
|||
private enum isSlicePointer(T) = isPointer!T || is(T : PtrShell!R, R); | |||
|
|||
private struct LikePtr {} | |||
package struct LikePtr {} |
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.
Why is this attribute needed? Doesn't capability introspection suffice?
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.
This is clear way and for internal use only. User defined ranges may brake introspection for pointer like behavior
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.
What if I want to implement my own algorithm that is not part of Phobos? It seems like I would want to be able to use LikePtr
in such a situation to interface seamlessly with the rest of ndslice.
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.
@klickverbot You can use random access range interface and sliced
function. @LikePtr
is for optimisation reasons. It may be exposed in the future, but I need to write a proper documentation of how it should be used first. The number of use cases for @LikePtr
is very small, and all of them seems should be a part of the package.
First, is it really nessesary for all of the planned stuff to be in this one PR? You could build up the module PR by PR in order to make this easier to review. Even though it might seem odd to have a module with only one function in it, that would only be temporary and that's fine for std.experimental. Secondly, I have a slight problem with the names of the planned functions. One of the main benefits of ndslice that I laid out in my article is that it feels like it's a part of the rest of the language because you don't have to use a whole bunch specialized functions in order to get the functionality you want. This is one of my main gripes with numpy: in your code you have numpy code, and then you have Python code, and they don't mix well. With these planned functions, instead of doing the natural thing |
I would also be curious about the rationale behind the |
No, only
Both |
@klickverbot As was wrote for @JackStouffer both map and ndmap are valid: Original matrix
|
The naming logic is:
|
Macros: | ||
SUBREF = $(REF_ALTTEXT $(TT $2), $2, std,experimental, ndslice, $1)$(NBSP) | ||
T2=$(TR $(TDNW $(LREF $1)) $(TD $+)) | ||
T4=$(TR $(TDNW $(LREF $1)) $(TD $2) $(TD $3) $(TD $4)) |
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.
nitpick: this is an unused macro
To put the following opinions into context, I'm a physicist by trade and rather more familiar with NumPy and Julia than I'd like, having also taught numerical programming classes before. I haven't had a chance to really familiarise myself with your ndslice work yet, though. First of all, I get that iteration over multidimensional slices is difficult to reconcile with simple ranges, especially for "structure-preserving" operations like How do I map over just some dimensions (e.g. rows or columns in a 2D matrix)?
I'll have to strongly disagree on that.
I'm missing the argument for breaking with Phobos conventions, which provide uniformity for users and hence decreases the cognitive overload, as they can guess the name (CamelCase). |
Looks great - I started to play with it & it feels exactly how I imagined it. Great job!
👍 from my side. Maybe you should add that further abstractions can be done with
This could be very confusing. Can't you have two overloads?
You can use packed slices, see my comment to the unittest ;-) |
ndslice is much more powerful then NumPy and Julia in case of dimension abstraction. It has zero cost abstraction for tensors composed of tensors (composed of tensors and etc). See the last example for |
I am open for alternative function names |
The reason that have different argument order: |
You need a random access range, not a RoR. |
Your range in not flat and it is not random access. |
Ok, seems like I need to allocate an array. Thanks. |
@klickverbot I have added
|
I recall there was quite a discussion on the name issue and I think @9il made a strong point for |
@wilzbach I think @JackStouffer above made the point originally and most succinctly.
Think about the impact on code refactoring. Want to change from slices to ndslices, now you have to change every place you use an std.algorithm function. Also, suppose you're writing some generic code that should work on any input range. This naming approach requires template specialization in order to get it work with ndslices. Personally, I would prefer what is in ndslice.algorithm to eventually be a part of std.algorithm. That way I can just import std.algorithm : map and go to town. Nevertheless, I think a those favoring naming things ndMap, etc, made a good point with respect to the fact that the behavior of map is different for ndMap. Thus, I think there should be some kind of way to toggle between them (perhaps there's a better way than what I suggested above). |
I agree, that would be quite sweet, but I think there is still the problem of multi-dimensional mapping of #!/usr/bin/env dub
/+ dub.sdl:
name "mir_test"
dependency "mir" version="~>0.16.0-beta2"
+/
void main() {
import mir.ndslice;
import std.stdio;
import std.algorithm;
auto a = iotaSlice(2, 3).ndMap!("a + a");
writeln(a); // [[1, 2, 3], [4, 5, 6]]
auto b = iotaSlice(2, 3).pack!1.ndMap!(a => a.maxPos.front);
writeln(b); // [2, 5]
} @9il what do you think about this? I think I can remember a discussion on a different PR where it was agreed that internally using |
This would not work for all other functions. API can be found here http://docs.mir.dlang.io/latest/mir_ndslice_algorithm.html |
#4781 contains |
PR is available in Mir >=v0.16.0-alpha7
Benchmarks: https://github.com/libmir/mir/tree/master/benchmarks/ndslice
Docs: http://docs.mir.dlang.io/latest/mir_ndslice_algorithm.html
Difference with
std.algorithm
ndMap
) out of the boxzip
analog -assumeSameStructure
, which is fasterzip
/assumeSameStructure
ndslice.algorithm
ndMap
(multidimensional map type ofSlice(N, xxx)
)ndFold
(single tensor, multiple functions and seeds)ndReduce
(single seed, multiple tensors)vectorization
andfastmath
flagsndEach
(no seed, multiple tensors)vectorization
andfastmath
flagsndAll
(multiple tensors)ndAny
(multiple tensors)ndFind
(multiple tensors) [reworked]ndCmp
(multidimensional propagation)ndEqual
(multiple tensors)ndCanFindNeedle
andndFindNeedle
was removed.ndslice.slice
ndFold
.opCmp
usingndCmp
ndslice.package
See Also
ldc-developers/ldc#1669
Asked Questions
You can use random access range interface and sliced function. @LikePtr is for optimisation reasons. It may be exposed in the future, but I need to write a proper documentation of how it should be used first. The number of use cases for @LikePtr is very small, and all of them seems should be a part of the package.
Yes, but not all "scalar" functions should be adopted. Current list of functions is enough.
For example:
byElement
to get random access range of all elements or make a copy/index.ndSwap(a, b)
because we can just dondEach!swap(a, b)
.ndCount
because it can be easily implemented withndReduce
.Yes, for performance reasons.
flattened
is calledbyElement
, it is selector operator.I add this module because performance reasons. N is dimension count:
byElement
requires additional computations :add
operations for forward access (empty, front, popFront).div
/mod
pairs plus 2*N-1 moreadd
for random and backward access.div
/mod
are expensive.byElement
requires N+1 additional general purpose registers. This is bad for iterating few matrixes in the lockstep.This is true only for Slices, which have continues in-memory representation. Numpy just copies all ndarray, if the ndarray can not be reshaped.
The result of
ndMap
is aSlice
. The result ofmap
is the range, and if we want to use ndslice module withmap
, we need tosliced
again. The real problem is thatbyElement
has slow random access primitive to build a slice on top :In the same time #4647 provides vectorised operations like
+=
, this PR also have vectorised algorithms. In addition, Mir will have BLAS soon, see current benchmark. Mir has the same performance for large matrixes as OpenBLAS, this means that Mir's generic gemm SIMD kernels are optimal. For small matrixes I need to write generic SIMD transposition kernels for packing, but this is small effort comparing with gemm SIMD kernels.I don't think so. ndslice extends our universe. We have different types of ranges. ndslice is just the next after random access range:
byElement
is random access range, with fast forward iteration (as much fast as possible with Range API).ndSwap
for example, because we can just writendEach!swap
. So, please do not think that I am rewriting std.algorithm: current functions provide only optimal loop construction with different stop criteria.