On Thursday, 3 December 2015 at 15:07:55 UTC, Andrei Alexandrescu
wrote:
On 12/01/2015 11:03 AM, Jack Stouffer wrote:
On Sunday, 29 November 2015 at 20:53:43 UTC, Jack Stouffer
wrote:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer
wrote:
This is the start of the two week formal review
Friendly reminder that the review ends tomorrow.
The two week review is over.
Thanks to all who participated in this. This is not a formal
review, but I have a few comments (some of which echo what
others said). They are not blocking, i.e. they shouldn't be
addressed before voting.
DOCUMENTATION
* A native English speaker should make a careful pass through
the documentation, there are a few simple things that are
likely to escape systematically to a non-native. E.g. "which
are represented with the Slice." -> "which are represented with
the Slice type."
I will create review with professional translator at least.
* There should be a synopsis at the very beginning of the
module with a short complete example illustrating what the
package can be used for.
Plans to add one example for image processing and one for
computer vision.
* The use of "bifacial" for template and non-template is new?
Yes
* sliced() doesn't explain what happens if the input size is
not a multiple of the sizes.
will fix
* Do createSlice() and ndarray() work with allocators?
Looks like you have read old docs. I have added a lot features
and split module into package. LOC growed from 1.5K to 4.5K
Quick brief of changes can be found in
https://github.com/D-Programming-Language/phobos/pull/3397 .
The latest docs (right now):
http://dtest.thecybershadow.net/artifact/website-8937f229ab6d7bffa1c5996673347d0071563ef1-44ccae0e926aef9268d7289ec985a497/web/phobos-prerelease/std_experimental_ndslice.html
ALLOCATORS:
1. There is no any kind off allocation (string concatenations in
asserts still needs to be checked) in updated
std.experimental.ndslice package except the `ReshapeException` in
the `reshape` function.
2. There is tiny example for `sliced` with `makeSlice` function,
which user can copy-past to work with allocators.
3. `ndarray` is an example too. To make it work with allocators
we need std.array.array to work with allocators first.
* Not a fan of non-imperative function names for imperative
functions, i.e. transposed(), swapped(), everted() etc. stand
out like so many sore thumbs. For example the description of
reversed() says "ReverseS direction of the iteration" so I
assume it's just doing it imperatively. In Phobos we generally
use action verbs for imperative actions and "-ed" for lazy
behavior. Would be nice to continue that.
OK, `*ed` will be removed.
Simplest 1D Example for reverse:
data: 0 1 2 3 4 5
^
ptr, stride = 1
after reverse:
data: 0 1 2 3 4 5
^
ptr, stride = -1
No functions change data in `ndslice` package.
In updated docs there is two kind of functions:
1. std.experimental.ndslice.iteration contains functions like
`transposed` and other `*ed` stuff (`*ed` will be removed).
1. This functions do _not_ change data
2. Have the _same_ return type
2. std.experimental.ndslice.selection contains functions like
`reshape, `blocks` and `byElement`.
1. This functions do _not_ change data
2. Have _another_ return type
* Why does byElement return just an input range (and not a more
powerful one)?
Fixed to Random Access Range with slicing.
Please use the link above to access updated docs. And PR to
access the latest docs via CyberShadow doc engine.
* Typo: std/experemental/range/ndslice.d
* Documentation is incomplete at the bottom of
std.experimental.range.ndslice, i.e. there are a bunch of names
without descriptions.
Partially fixed, still waiting for my translator
* Why is typeof((new int [1000].sliced(5, 6, 7)) Slice!(3,
int*) and not Slice!(3, int[])? That seems to suggest there's
little @safety in the interface.
`ReplaceArrayWithPointer` flag was added. By default it is
`true`. It can be used for CTFE-able code. Performance difference
between pointer and array is very signifiant:
Binary structure with pointer:
lengths
strides
ptr
Binary structure with array:
lengths
strides
array (pointer + length)
shift (shift to the front element in a slice)
It is impossible to have flexible engine for arrays/ranges
without `shift`. Pointers have not such constraints.
IMPLEMENTATION
[...]
Very nice work. Thanks!
Andrei
Thank you for review!
Ilya
