www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Formal Review of std.range.ndslice

reply Jack Stouffer <jack jackstouffer.com> writes:
This is the start of the two week formal review for the proposed 
std.range.ndslice. This new addition to the standard library 
would add the ability to create and manipulate multi-dimensional 
random access ranges in a way that will be very familiar to those 
of you who use numpy. This has the potential to give D a huge 
boost in popularity in numerical and scientific applications.

A quick run down for those that are not familiar with the 
process. After two weeks, the PR author (Ilya Yaroshenko) will 
have time to make proposed changes. Then, when the author feels 
it's ready, the PR will go to a vote. In the vote, everyone in 
the community has a say, but if one of the main contributors or 
maintainers has a very negative opinion (for example) that will 
carry more weight.

Github: https://github.com/D-Programming-Language/phobos/pull/3397
dub: http://code.dlang.org/packages/dip80-ndslice
docs: 
http://dtest.thecybershadow.net/results/bac6211c1d73b2cf62bc18c9844c8c82c92c21e1/5c6071ca953cf113febd8786b4b68916cbb2cdaf/

previous discussion: 
http://forum.dlang.org/thread/rilfmeaqkailgpxoziuo forum.dlang.org
Nov 16 2015
next sibling parent reply Brian Schott <briancschott gmail.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard 
 library would add the ability to create and manipulate 
 multi-dimensional random access ranges in a way that will be 
 very familiar to those of you who use numpy. This has the 
 potential to give D a huge boost in popularity in numerical and 
 scientific applications.
One thing that stands out is the number of symbols in the Slice struct that have simple "///" doc comments. That is, they show up in the generated documentation without any explanation.
Nov 16 2015
parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Tuesday, 17 November 2015 at 00:10:42 UTC, Brian Schott wrote:
 On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer 
 wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard 
 library would add the ability to create and manipulate 
 multi-dimensional random access ranges in a way that will be 
 very familiar to those of you who use numpy. This has the 
 potential to give D a huge boost in popularity in numerical 
 and scientific applications.
One thing that stands out is the number of symbols in the Slice struct that have simple "///" doc comments. That is, they show up in the generated documentation without any explanation.
Thanks! Fixed http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-2b25fcf8789289fb52e50e2e71914d1a/web/phobos-prerelease/std_experimental_range_ndslice.html --Ilya
Nov 17 2015
prev sibling next sibling parent reply =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard
Why can't byElement http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-2b25fcf8789289fb52e50e2e71914d1a/web/phobos-prerelease/std_experimental_range_ndslice.html#.byElement provide RandomAccess? It's documented to return an InputRange.
Nov 17 2015
parent reply =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Tuesday, 17 November 2015 at 10:13:57 UTC, Nordlöw wrote:
 Why can't byElement

 http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-2b25fcf8789289fb52e50e2e71914d1a/web/phobos-prerelease/std_experimental_range_ndslice.html#.byElement

 provide RandomAccess? It's documented to return an InputRange.
AFAIK, if all the slice dimensions are know at compile-time it should. This is kind of similar to how std.range.chain() works.
Nov 17 2015
parent reply Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Tuesday, 17 November 2015 at 10:16:56 UTC, Nordlöw wrote:
 On Tuesday, 17 November 2015 at 10:13:57 UTC, Nordlöw wrote:
 Why can't byElement

 http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-2b25fcf8789289fb52e50e2e71914d1a/web/phobos-prerelease/std_experimental_range_ndslice.html#.byElement

 provide RandomAccess? It's documented to return an InputRange.
AFAIK, if all the slice dimensions are know at compile-time it should. This is kind of similar to how std.range.chain() works.
Thanks! Implemented: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-4f243752aee5ecc098ce1c36eedd86a2/web/phobos-prerelease/std_experimental_range_ndslice.html#byElement --Ilya
Nov 17 2015
next sibling parent =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Tuesday, 17 November 2015 at 18:38:14 UTC, Ilya Yaroshenko 
wrote:
 Thanks! Implemented: 
 http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-4f243752aee5ecc098ce1c36eedd86a2/web/phobos-prerelease/std_experimental_range_ndslice.html#byElement

 --Ilya
Fantastic! Thx, for the quick response.
Nov 18 2015
prev sibling parent reply =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Tuesday, 17 November 2015 at 18:38:14 UTC, Ilya Yaroshenko 
wrote:
 AFAIK, if all the slice dimensions are know at compile-time it 
 should. This is kind of similar to how std.range.chain() works.
Thanks! Implemented: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-4f243752aee5ecc098ce1c36eedd86a2/web/phobos-prerelease/std_experimental_range_ndslice.html#byElement --Ilya
I don't see any other checks using `isRandomAccessRange`. Surely there must be other ranges/algorithms in ndslice that should propagate random access, right?
Nov 18 2015
parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Wednesday, 18 November 2015 at 10:18:37 UTC, Nordlöw wrote:
 On Tuesday, 17 November 2015 at 18:38:14 UTC, Ilya Yaroshenko 
 wrote:
 AFAIK, if all the slice dimensions are know at compile-time 
 it should. This is kind of similar to how std.range.chain() 
 works.
Thanks! Implemented: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-4f243752aee5ecc098ce1c36eedd86a2/web/phobos-prerelease/std_experimental_range_ndslice.html#byElement --Ilya
I don't see any other checks using `isRandomAccessRange`. Surely there must be other ranges/algorithms in ndslice that should propagate random access, right?
Only Slice and ByElements has random access. Special check was added for internal PtrShell, however it is commented with `version(none)` because it works only with git compiler (I have fixed std.internal.test.dummyrange ). There was check using `isRandomAccessRange` after range primitives in Slice. I have added `hasLength` and `hasSlicing` too https://github.com/D-Programming-Language/phobos/commit/919009dce8903eca1ded119036ed5b2dd5bfcaa5 Updated docs: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-3429854b54bfa5eb7060de221b45056f/web/phobos-prerelease/std_experimental_range_ndslice.html -- Ilya
Nov 18 2015
prev sibling next sibling parent reply =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 docs: 
 http://dtest.thecybershadow.net/results/bac6211c1d73b2cf62bc18c9844c8c82c92c21e1/5c6071ca953cf113febd8786b4b68916cbb2cdaf/
There seems to be a mis-spelling of experimental as experemental: Search for: std/experemental/range/ndslice.d
Nov 17 2015
parent =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Tuesday, 17 November 2015 at 10:19:26 UTC, Nordlöw wrote:
 There seems to be a mis-spelling of experimental as 
 experemental:
Here: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-2b25fcf8789289fb52e50e2e71914d1a/web/phobos-prerelease/std_experimental_range_ndslice.html
Nov 17 2015
prev sibling next sibling parent Jack Stouffer <jack jackstouffer.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 docs: 
 http://dtest.thecybershadow.net/results/bac6211c1d73b2cf62bc18c9844c8c82c92c21e1/5c6071ca953cf113febd8786b4b68916cbb2cdaf/
I have already posted some things on Github, so If you will indulge me signing the praises of this addition for a moment: Something dawned on me when I was scanning the numpy documentation, looking for anything obvious that was missed by this PR. What I noticed is that D really made the right bet when it came to putting a lot of range code in the standard library, because in order to make numpy useful at all, the authors had to recreate parts of the Python standard library in their C code so that it takes advantage of the type information that numpy arrays provide, where as all of std.algorithm and std.range can be used with this no problem. For example to create a zero initialized n-d array in numpy, you would have to use the numpy function np.zeros((10, 5)), but with this you can repeat(0, 50).sliced(10, 5) with no performance penalty. In numpy to take the sum of a certain axis, you can use Python's len or sum like sum(array[:, 0]) but it's slow so you have to remember the special case that you need to use array[:, 0].sum() instead to take advantage of numpy's types. With this it's just array[0..$, 0].sum with the same function you always use in the way you always use it. There are a ton of these special cases, and the consequence is it makes it very hard for non numpy code and numpy code to mix in a way that doesn't incur penalties or is a huge pain. Also, this is a great showcase for how powerful D templates are as this solution is entirely a library solution where as numpy uses mountains of C code glued to Python. So, I think that this can be more powerful than numpy, as this fits naturally into the rest of your code base and can work efficiently with any range based code already written.
Nov 17 2015
prev sibling next sibling parent reply Robert burner Schadek <rburners gmail.com> writes:
I couldn't easily find how to make the module work with 
allocators. IMO combining this module with 
std.experiemtal.allocator should be possible. And if it is 
already possible, there should be tests for documentation and 
validation.
Nov 18 2015
next sibling parent John Colvin <john.loughran.colvin gmail.com> writes:
On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
Schadek wrote:
 I couldn't easily find how to make the module work with 
 allocators. IMO combining this module with 
 std.experiemtal.allocator should be possible. And if it is 
 already possible, there should be tests for documentation and 
 validation.
https://github.com/D-Programming-Language/phobos/pull/3397#issuecomment-157652959
Nov 18 2015
prev sibling next sibling parent reply Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
Schadek wrote:
 I couldn't easily find how to make the module work with 
 allocators. IMO combining this module with 
 std.experiemtal.allocator should be possible. And if it is 
 already possible, there should be tests for documentation and 
 validation.
I have added makeSlice, however this function can be easily implemented by user: allocate array -> pass array to `sliced`. updated docs http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-ce47155797f387348826317811c4af0c/web/phobos-prerelease/std_experimental_range_ndslice.html --Ilya
Nov 18 2015
parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Wednesday, 18 November 2015 at 10:26:13 UTC, Ilya Yaroshenko 
wrote:
 On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
 Schadek wrote:
 I couldn't easily find how to make the module work with 
 allocators. IMO combining this module with 
 std.experiemtal.allocator should be possible. And if it is 
 already possible, there should be tests for documentation and 
 validation.
I have added makeSlice, however this function can be easily implemented by user: allocate array -> pass array to `sliced`. updated docs http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-ce47155797f387348826317811c4af0c/web/phobos-prerelease/std_experimental_range_ndslice.html --Ilya
I spoke with Andrei over email and his opinion was that if this module makes any allocation that can't be avoided, then std.experimental.allocator should be integrated.
Nov 20 2015
parent reply Ilya <ilyayaroshenko gmail.com> writes:
On Friday, 20 November 2015 at 19:21:45 UTC, Jack Stouffer wrote:
 On Wednesday, 18 November 2015 at 10:26:13 UTC, Ilya Yaroshenko 
 wrote:
 On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
 Schadek wrote:
 [...]
I have added makeSlice, however this function can be easily implemented by user: allocate array -> pass array to `sliced`. updated docs http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-ce47155797f387348826317811c4af0c/web/phobos-prerelease/std_experimental_range_ndslice.html --Ilya
I spoke with Andrei over email and his opinion was that if this module makes any allocation that can't be avoided, then std.experimental.allocator should be integrated.
Great! Added to PR's TODO list :-)
Nov 20 2015
parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Friday, 20 November 2015 at 20:33:27 UTC, Ilya wrote:
 Great! Added to PR's TODO list :-)
This will be interesting because there is no defined idiomatic usage of std.allocator. I imagine std.range.ndslice will be the trend-setter for Phobos in this regard.
Nov 20 2015
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/15 4:23 PM, Jack Stouffer wrote:
 On Friday, 20 November 2015 at 20:33:27 UTC, Ilya wrote:
 Great! Added to PR's TODO list :-)
This will be interesting because there is no defined idiomatic usage of std.allocator. I imagine std.range.ndslice will be the trend-setter for Phobos in this regard.
There's the persistent list example that I posted on dpaste a couple weeks back. I do expect things to evolve a bit before stabilizing. BTW thanks Jack for reaching out via email. Andrei
Nov 20 2015
parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Friday, 20 November 2015 at 21:55:18 UTC, Andrei Alexandrescu 
wrote:
 On 11/20/15 4:23 PM, Jack Stouffer wrote:
 On Friday, 20 November 2015 at 20:33:27 UTC, Ilya wrote:
 Great! Added to PR's TODO list :-)
This will be interesting because there is no defined idiomatic usage of std.allocator. I imagine std.range.ndslice will be the trend-setter for Phobos in this regard.
There's the persistent list example that I posted on dpaste a couple weeks back. I do expect things to evolve a bit before stabilizing. BTW thanks Jack for reaching out via email. Andrei
After playing with memory allocation I became convinced that ndslice package should not have any kind of memory allocation (except opCast to array) at least we will add allocators support to std.array. The most important function is std.array.array. Out of the box user can do `auto slice = new int[2*3*4+n].sliced(2, 3, 4); //n>=0`. The link below contains convenience functions in examples for `sliced` 1. `createSlice` creates an GC allocated slice 2. `ndarray` creates GC allocated ndarray copy of a slice 3. `makeSlice` make slice with allocators They are only examples and not a part of the API. http://dtest.thecybershadow.net/artifact/website-20b34f54aaeb876b545bac1d34b954db15f0a237-0d1a48cd3d6cc5a31c97ba84a93229e4/web/phobos-prerelease/std_experimental_ndslice_slice.html#.sliced --Ilya
Nov 21 2015
prev sibling parent Ilya <ilyayaroshenko gmail.com> writes:
On Friday, 20 November 2015 at 21:23:40 UTC, Jack Stouffer wrote:
 On Friday, 20 November 2015 at 20:33:27 UTC, Ilya wrote:
 Great! Added to PR's TODO list :-)
This will be interesting because there is no defined idiomatic usage of std.allocator. I imagine std.range.ndslice will be the trend-setter for Phobos in this regard.
Slice is only a shell over a range/array. There is no reason to add any kind of counters or special stuff entities. All this idioms work out of the box because Slice holds an array. It is already works with std.container.array, and it will work with the future Array container based on allocators. -- Ilya
Nov 20 2015
prev sibling parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
Schadek wrote:
 I couldn't easily find how to make the module work with 
 allocators. IMO combining this module with 
 std.experiemtal.allocator should be possible. And if it is 
 already possible, there should be tests for documentation and 
 validation.
I know it's bad practice for regular Phobos code to import from std.experimental, but what's the protocol for code inside std.experimental importing other code in std.experimental? It would be nice to get Andrei's opinion on this. Another problem with using std.experimental.allocator is it ties the time table of this going into stable with std.allocator. Worse case scenario, it could be a year before std.allocator is stable, where as std.range.ndslice is much simpler code and could be moved much faster.
Nov 18 2015
parent reply Ilya <ilyayaroshenko gmail.com> writes:
On Wednesday, 18 November 2015 at 18:40:40 UTC, Jack Stouffer 
wrote:
 On Wednesday, 18 November 2015 at 08:52:11 UTC, Robert burner 
 Schadek wrote:
 [...]
I know it's bad practice for regular Phobos code to import from std.experimental, but what's the protocol for code inside std.experimental importing other code in std.experimental? It would be nice to get Andrei's opinion on this. Another problem with using std.experimental.allocator is it ties the time table of this going into stable with std.allocator. Worse case scenario, it could be a year before std.allocator is stable, where as std.range.ndslice is much simpler code and could be moved much faster.
Possible solutions: 1. Mark `makeSlice` with red "Experimental" 2. Move `makeSlice` to unittest, so user could copy-past it.
Nov 18 2015
parent Jack Stouffer <jack jackstouffer.com> writes:
On Wednesday, 18 November 2015 at 19:25:04 UTC, Ilya wrote:
 Possible solutions:
 1. Mark `makeSlice` with red "Experimental"
 2. Move `makeSlice` to unittest, so user could copy-past it.
Another solution is to leave `makeSlice` in std.experimental when std.experimental.range.ndslice is moved to stable and move it to std.range when std.experimental.allocator is moved. I think it would be best to get someone from the leadership's opinions on this before a decision is made.
Nov 18 2015
prev sibling next sibling parent reply Lucien Janvier <lulu meh.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard 
 library would add the ability to create and manipulate 
 multi-dimensional random access ranges in a way that will be 
 very familiar to those of you who use numpy. This has the 
 potential to give D a huge boost in popularity in numerical and 
 scientific applications.

 [...]
I don't see any constraint related to dimenssion checking.
Nov 18 2015
parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Wednesday, 18 November 2015 at 20:10:00 UTC, Lucien Janvier 
wrote:
 On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer 
 wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard 
 library would add the ability to create and manipulate 
 multi-dimensional random access ranges in a way that will be 
 very familiar to those of you who use numpy. This has the 
 potential to give D a huge boost in popularity in numerical 
 and scientific applications.

 [...]
I don't see any constraint related to dimenssion checking.
I have add description to asserts https://github.com/9il/phobos/commit/f91d611ad66af9b6698142f92bcb8cc05747aff0 Is it constraints you asked?
Nov 18 2015
prev sibling next sibling parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Monday, 16 November 2015 at 22:45:35 UTC, Jack Stouffer wrote:
 This is the start of the two week formal review for the 
 proposed std.range.ndslice. This new addition to the standard 
 library would add the ability to create and manipulate 
 multi-dimensional random access ranges in a way that will be 
 very familiar to those of you who use numpy. This has the 
 potential to give D a huge boost in popularity in numerical and 
 scientific applications.

 A quick run down for those that are not familiar with the 
 process. After two weeks, the PR author (Ilya Yaroshenko) will 
 have time to make proposed changes. Then, when the author feels 
 it's ready, the PR will go to a vote. In the vote, everyone in 
 the community has a say, but if one of the main contributors or 
 maintainers has a very negative opinion (for example) that will 
 carry more weight.

 Github: 
 https://github.com/D-Programming-Language/phobos/pull/3397
 dub: http://code.dlang.org/packages/dip80-ndslice
 docs: 
 http://dtest.thecybershadow.net/results/bac6211c1d73b2cf62bc18c9844c8c82c92c21e1/5c6071ca953cf113febd8786b4b68916cbb2cdaf/

 previous discussion: 
 http://forum.dlang.org/thread/rilfmeaqkailgpxoziuo forum.dlang.org
## Guide for Slice/Matrix/BLAS contributors 1. Pay _unprecedented_ attention to functions to be a. inlined(!), b. ` nogc`, c. `nothrow`, d. `pure`. 95% of functions will be marked with `pragma(inline, true)`. So, use _simple_ `assert`s with messages that can be computed at compile time. The goals are: 1. to reduce executable size for _any_ compilation mode 2. to reduce template bloat in object files 3. to reduce compilation time 4. to allow a user to write an extern C bindings for code based on `Slice`. 2. Do not import `std.format`, `std.string` and `std.conv` to format error messages.`"Use" ~ Concatenation.stringof ~ ", really ."` Why? Please, read [1] again. 3. Try to use already defined `mixin template`s for pretty error messaging. 4. Do not use `Exception`s/`enforce`s to check indexes and length. Exceptions are allowed only for algorithms where validation of an input data is significantly complex for user. `reshaped` is a good example where Exceptions are required. Put an example of Exception handing and workaround for function that can throw. 5. Do not use compile time flags for simple checks like transposition of a matrix. It is much better to have runtime matrix transposition. Furthermore, Slice provide runtime matrix transposition out of the box. 6. Do not use _Fortran_VS_C_ flags. They are about notation, but not about algorithm itself. To care about math world users add appropriate code example in the documentation. `transposed` / `everted` can be used for cash friendly code. 7. Do not use compile time power of D to produce dummy entities like `IdentityMatrix`. 8. Try to separate allocation and algorithm logic whenever possible. 9. Add CTFE unittests to new functions. --- Update docs: http://dtest.thecybershadow.net/artifact/website-7a646fdea76569e009844cdee5c93edab10980ca-87c7c7a1a6e59a71179301c54d012057/web/phobos-prerelease/std_experimental_range_ndslice.html -- Ilya
Nov 20 2015
prev sibling next sibling parent reply Jack Stouffer <jack jackstouffer.com> writes:
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.
Nov 29 2015
parent reply Jack Stouffer <jack jackstouffer.com> writes:
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. Thank you to everyone who commented here or on the PR. Of course, even though the review is over, you can still make comments on the GitHub PR up until it's merged. 9il, please let me know if you want to start the voting right away or wait until your list is completed.
Dec 01 2015
next sibling parent Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Tuesday, 1 December 2015 at 16:03:51 UTC, 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. Thank you to everyone who commented here or on the PR. Of course, even though the review is over, you can still make comments on the GitHub PR up until it's merged. 9il, please let me know if you want to start the voting right away or wait until your list is completed.
Thank you, Jack! I plan to check English text and add 4D example for image processing and 5D example for computer vision first. They should help users to imagine use cases. Plus looks like we need comparison between D.Slice and numpy.ndarray. Slice is more flexible, faster and generalised comparing with ndarray. In the same time Slice has not problems with extensions like numpy dose (as you have already noted) and it has tiny (in LOC) implementation. I hope that examples and the comparison would be moved later to DBlog (D needs blog!) with detailed explanation by another author (I can be reviewer).
Dec 01 2015
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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." * 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. * The use of "bifacial" for template and non-template is new? * sliced() doesn't explain what happens if the input size is not a multiple of the sizes. * Do createSlice() and ndarray() work with allocators? * 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. * Why does byElement return just an input range (and not a more powerful one)? * 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. * 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. IMPLEMENTATION * According to git grep, in phobos there are 8139 occurrences of 'if (' and 2451 occurrences of 'if('. We should avoid such jitters in new code. Please change if/while/for/foreach/etc to include a space before the paren (actually some do have the space, others don't). There are other style violations as well, e.g. no space around operators (but only sometimes, inconsistently) etc. For existing code it's a slow process but this is new code and this is the right time to make it flush with the overall phobos style. * The code should use "private" appropriately. * Code should use local imports wherever possible. * There's some commented-out code there, shouldn't. * Duplication easy to eliminate between isPermutation and isPartialPermutation, just have the first call the second and then do the extra work. * I think we have something good for isReference/hasReference in std.traits. * Unittests should use safe wherever possible, which may prompt changes in the interface and implementation. Very nice work. Thanks! Andrei
Dec 03 2015
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 12/03/2015 10:07 AM, Andrei Alexandrescu wrote:
 * The code should use "private" appropriately.
And "package" etc. -- Andrei
Dec 03 2015
prev sibling parent reply Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
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
Dec 03 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 12/03/2015 11:50 AM, Ilya Yaroshenko wrote:
 No functions change data in `ndslice` package.
Then probably "-ed" is appropriate. -- Andrei
Dec 03 2015
prev sibling parent reply Stefan Frijters <sfrijters gmail.com> writes:
Today I've made an abortive attempt at replacing my code's [1] 
dependence on unstd.multidimarray [2] with ndslice.
I'm guessing it's just me being stupid, but could anyone supply 
with some hints on how to do the conversion with a minimum of 
fuss?

Basically I have an N-dimensional array (N is known at compile 
time) of type T wrapped in a struct to carry some additional 
information.
I then address it using size_t[N] fixed-size arrays, and loop 
over it a lot with foreach, which also uses size_t[N] as index.

So it looks something like this:

struct Field(T, uint N) {

   alias arr this;

   MultidimArray!(T, N) arr; // is there any way to supply the 
correct type here with ndslice? I cannot use auto, right?

   this (in size_t[N] lengths) {
     arr = multidimArray!T(lengths);
   }
}
and then things like

foreach(immutable p, ref pop; someField) {
   pop = foo(someOtherField[bar(p)]);
   ...
}

where p is of type size_t[N].

I tried using ndarray in conjunction with the 
std.experimental.allocator, but I don't particularly care about 
memory management;
these are large arrays that are allocated once and kept around 
for the duration of the program.

Any help would be appreciated.

[1] https://github.com/SFrijters/DLBC
[2] https://bitbucket.org/SFrijters/unstandard
Dec 11 2015
parent reply Ilya <ilyayaroshenko gmail.com> writes:
On Friday, 11 December 2015 at 19:31:14 UTC, Stefan Frijters 
wrote:
 Today I've made an abortive attempt at replacing my code's [1] 
 dependence on unstd.multidimarray [2] with ndslice.
 I'm guessing it's just me being stupid, but could anyone supply 
 with some hints on how to do the conversion with a minimum of 
 fuss?

 Basically I have an N-dimensional array (N is known at compile 
 time) of type T wrapped in a struct to carry some additional 
 information.
 I then address it using size_t[N] fixed-size arrays, and loop 
 over it a lot with foreach, which also uses size_t[N] as index.

 So it looks something like this:

 struct Field(T, uint N) {

   alias arr this;

   MultidimArray!(T, N) arr; // is there any way to supply the 
 correct type here with ndslice? I cannot use auto, right?
Slice!(N, T*) arr;
   this (in size_t[N] lengths) {
     arr = multidimArray!T(lengths);
// compute length // more flexible construtors would be added after // allocatrs support for ndslice size_t len = 1; foreach(l; lengths) len *= l; arr = new T[len].sliced(lengths);
   }
 }
 and then things like

 foreach(immutable p, ref pop; someField) {
   pop = foo(someOtherField[bar(p)]);
   ...
 }
std.experimental.ndslice.selection: indexSlice, byElement; foreach(p; someField.shape.indexSlice.byElement) { someField[p] = foo(someOtherField[bar(p)]); ... }
 where p is of type size_t[N].

 I tried using ndarray in conjunction with the 
 std.experimental.allocator, but I don't particularly care about 
 memory management;
 these are large arrays that are allocated once and kept around 
 for the duration of the program.

 Any help would be appreciated.

 [1] https://github.com/SFrijters/DLBC
 [2] https://bitbucket.org/SFrijters/unstandard
See also updated docs: http://dtest.thecybershadow.net/artifact/website-13cbdcf17d84fc31328c3f517a56bea783c418d6-d9c63e815273f0906309088334e7dfb1/web/phobos-prerelease/std_experimental_ndslice.html Ilya
Dec 11 2015
next sibling parent Ilya <ilyayaroshenko gmail.com> writes:
On Friday, 11 December 2015 at 22:56:15 UTC, Ilya wrote:
 On Friday, 11 December 2015 at 19:31:14 UTC, Stefan Frijters 
 wrote:
 Today I've made an abortive attempt at replacing my code's [1] 
 dependence on unstd.multidimarray [2] with ndslice.
 I'm guessing it's just me being stupid, but could anyone 
 supply with some hints on how to do the conversion with a 
 minimum of fuss?

 Basically I have an N-dimensional array (N is known at compile 
 time) of type T wrapped in a struct to carry some additional 
 information.
 I then address it using size_t[N] fixed-size arrays, and loop 
 over it a lot with foreach, which also uses size_t[N] as index.

 So it looks something like this:

 struct Field(T, uint N) {

   alias arr this;

   MultidimArray!(T, N) arr; // is there any way to supply the 
 correct type here with ndslice? I cannot use auto, right?
Slice!(N, T*) arr;
   this (in size_t[N] lengths) {
     arr = multidimArray!T(lengths);
// compute length // more flexible construtors would be added after // allocatrs support for ndslice size_t len = 1; foreach(l; lengths) len *= l; arr = new T[len].sliced(lengths);
   }
 }
 and then things like

 foreach(immutable p, ref pop; someField) {
   pop = foo(someOtherField[bar(p)]);
   ...
 }
std.experimental.ndslice.selection: indexSlice, byElement; foreach(p; someField.shape.indexSlice.byElement) { someField[p] = foo(someOtherField[bar(p)]); ... }
faster version: std.experimental.ndslice.selection: byElement; for(auto r = someField.arr.byEleemnt; r.popFront) { r.front = foo(someOtherField[bar(r.index)]); ... }
 where p is of type size_t[N].
Ilya
Dec 11 2015
prev sibling parent reply Stefan Frijters <sfrijters gmail.com> writes:
On Friday, 11 December 2015 at 22:56:15 UTC, Ilya wrote:
 On Friday, 11 December 2015 at 19:31:14 UTC, Stefan Frijters 
 wrote:
 [...]
Slice!(N, T*) arr;
     [...]
// compute length // more flexible construtors would be added after // allocatrs support for ndslice size_t len = 1; foreach(l; lengths) len *= l; arr = new T[len].sliced(lengths);
 [...]
std.experimental.ndslice.selection: indexSlice, byElement; foreach(p; someField.shape.indexSlice.byElement) { someField[p] = foo(someOtherField[bar(p)]); ... }
 [...]
See also updated docs: http://dtest.thecybershadow.net/artifact/website-13cbdcf17d84fc31328c3f517a56bea783c418d6-d9c63e815273f0906309088334e7dfb1/web/phobos-prerelease/std_experimental_ndslice.html Ilya
Thank you for your help. I'm trying to convert my code again at the moment, but ran into a new problem: I need to pass a pointer to the data into a C function. It seems that the .ptr property is not available, and using & caused dmd to segfault (currently running a Dustmite reduction for that). Is there any clean way to get a pointer to the underlying data?
Dec 13 2015
parent reply Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Sunday, 13 December 2015 at 15:25:11 UTC, Stefan Frijters 
wrote:
 On Friday, 11 December 2015 at 22:56:15 UTC, Ilya wrote:
 On Friday, 11 December 2015 at 19:31:14 UTC, Stefan Frijters 
 wrote:
 [...]
Slice!(N, T*) arr;
     [...]
// compute length // more flexible construtors would be added after // allocatrs support for ndslice size_t len = 1; foreach(l; lengths) len *= l; arr = new T[len].sliced(lengths);
 [...]
std.experimental.ndslice.selection: indexSlice, byElement; foreach(p; someField.shape.indexSlice.byElement) { someField[p] = foo(someOtherField[bar(p)]); ... }
 [...]
See also updated docs: http://dtest.thecybershadow.net/artifact/website-13cbdcf17d84fc31328c3f517a56bea783c418d6-d9c63e815273f0906309088334e7dfb1/web/phobos-prerelease/std_experimental_ndslice.html Ilya
Thank you for your help. I'm trying to convert my code again at the moment, but ran into a new problem: I need to pass a pointer to the data into a C function. It seems that the .ptr property is not available, and using & caused dmd to segfault (currently running a Dustmite reduction for that). Is there any clean way to get a pointer to the underlying data?
Could you please post reduced code example that caused dmd to segfault? 2D way: &slice[0, 0] or &(slice.front.front()); ND way: &(slice.byElement.front()) Note: Comparing with unstandard there is no guarantee that the first element in a ndarray is the first element in memory. `reversed` and `allReversed` should not be used to preserve strides positive. The latests docs (with fixed English): http://dtest.thecybershadow.net/artifact/website-13cbdcf17d84fc31328c3f517a56bea783c418d6-dd2292a424959b594956eeeba64d391f/web/phobos-prerelease/std_experimental_ndslice.html Voting For std.experimental.ndslice http://forum.dlang.org/post/ztaudqxmjrgnilsioyqs forum.dlang.org Please Vote! Ilya
Dec 13 2015
parent reply Stefan Frijters <sfrijters gmail.com> writes:
On Sunday, 13 December 2015 at 15:59:19 UTC, Ilya Yaroshenko 
wrote:
 Could you please post reduced code example that caused dmd to 
 segfault?
Took dustmite about 6 hours to reduce, and then I went at it manually for a bit, so this is the smallest I could get it: import std.experimental.ndslice; int main() { Field force; foreach(p, e; force) e; } struct Field { alias arr this; Slice!(3, double*) arr; } Compiled with dmd 2.069.1 via dub build: { "name": "dlbc", "sourcePaths": ["src"], "dependencies": { "dip80-ndslice": "~>0.8.3", }, } dip80-ndslice 0.8.3: target for configuration "library" is up to date. dlbc ~master: building configuration "application"... Segmentation fault dmd failed with exit code 139. Since it's a segfault in the compiler, should I put it on Bugzilla too?
 2D way: &slice[0, 0]   or   &(slice.front.front());

 ND way: &(slice.byElement.front())

 Note: Comparing with unstandard  there is no guarantee that the 
 first element in a ndarray is the first element in memory. 
 `reversed` and `allReversed` should not be used to preserve 
 strides positive.
Hm, I assumed the underlying array would be a single block of data and then a bunch of pointers would be used to keep track of any slices. I'll try to figure out how to give the data to C then (for MPI and HDF5, to be exact).
Dec 13 2015
parent reply Ilya Yaroshenko <ilyayaroshenko gmail.com> writes:
On Sunday, 13 December 2015 at 21:24:36 UTC, Stefan Frijters 
wrote:
 On Sunday, 13 December 2015 at 15:59:19 UTC, Ilya Yaroshenko 
 wrote:
 Could you please post reduced code example that caused dmd to 
 segfault?
Took dustmite about 6 hours to reduce, and then I went at it manually for a bit, so this is the smallest I could get it: import std.experimental.ndslice; int main() { Field force; foreach(p, e; force) e; } struct Field { alias arr this; Slice!(3, double*) arr; } Compiled with dmd 2.069.1 via dub build: { "name": "dlbc", "sourcePaths": ["src"], "dependencies": { "dip80-ndslice": "~>0.8.3", }, }
More reduced code: import std.experimental.ndslice; void main() { Slice!(3, double*) force = new double[60].sliced(3, 4, 5); // Correct code. Compiles! for(auto r = force.byElement; !r.empty; r.popFront) { size_t[3] p = r.index; double e = r.front; } // Wrong foreach params. dmd failed with exit code -11. //foreach(p, e; force) //{ //} } dub.json { "name": "dlbc", "dependencies": { "dip80-ndslice": "~>0.8.3", }, }
 dip80-ndslice 0.8.3: target for configuration "library" is up 
 to date.
 dlbc ~master: building configuration "application"...
 Segmentation fault
 dmd failed with exit code 139.

 Since it's a segfault in the compiler, should I put it on 
 Bugzilla too?
Yes 1. Please note in Bugzilla report that program code is incorrect (see example of correct above). 2. More reduced code can be used for report: void main() { Slice!(3, double*) force = new double[60].sliced(3, 4, 5); // Wrong foreach params. dmd failed with exit code -11. foreach(p, e; force) { } }
 2D way: &slice[0, 0]   or   &(slice.front.front());

 ND way: &(slice.byElement.front())

 Note: Comparing with unstandard  there is no guarantee that 
 the first element in a ndarray is the first element in memory. 
 `reversed` and `allReversed` should not be used to preserve 
 strides positive.
Hm, I assumed the underlying array would be a single block of data and then a bunch of pointers would be used to keep track of any slices. I'll try to figure out how to give the data to C then (for MPI and HDF5, to be exact).
Probably you may need something like that: auto ar = new double[60]; //remember ar auto slice = ar.sliced(3, 4, 5); However, if you have not changed slice's structure (except transposition), then assert (ar == (&(slice.byElement.front()))[0..slice.elementsCount]); Ilya
Dec 13 2015
parent Stefan Frijters <sfrijters gmail.com> writes:
On Sunday, 13 December 2015 at 22:32:43 UTC, Ilya Yaroshenko 
wrote:
 Since it's a segfault in the compiler, should I put it on 
 Bugzilla too?
Yes 1. Please note in Bugzilla report that program code is incorrect (see example of correct above). 2. More reduced code can be used for report: void main() { Slice!(3, double*) force = new double[60].sliced(3, 4, 5); // Wrong foreach params. dmd failed with exit code -11. foreach(p, e; force) { } }
Reported as https://issues.dlang.org/show_bug.cgi?id=15441 . Will look into my other non-bug problems when I can, have to run now...
Dec 14 2015