Skip to content
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

std.experimental.ndslice [ready to merge] #3397

Merged
merged 1 commit into from Jan 2, 2016

Conversation

Projects
None yet
@9il
Copy link
Member

commented Jun 9, 2015

Coverage Status

DUB package is available at Mir library: link
Documentation: link
Voting thread: http://forum.dlang.org/thread/nexiojzouxtawdwnlfvt@forum.dlang.org
Review thread: http://forum.dlang.org/thread/uesnmkgniumswfclwbgt@forum.dlang.org
Old announce at forum.

Optimisation check

Source: https://gist.github.com/9il/bc7966823d96557c566c
LDC disassembled: https://gist.github.com/9il/47aea1621a9fba609869 (all functions are inlined!)
DMD disassembled: https://gist.github.com/9il/a5c0ee9bdb4ddd25c4d6

TODO list

  • rename module to std.experimental.ndslice and split.
  • module description
  • unroll dimension loops
  • more CT/RT checks
  • better error messages
  • make byElement a random access range
  • make byElement a range with slicing
  • add guide for contributors
  • add variadic template for strided
  • add reshape
  • add CTFE-able Slice over arrays
  • add diagonal Slice
  • add blocks Slice
  • add rotated Slice
  • add index property to all by*Element Iterators
  • add byElementInStandardSimplex
  • add windows - moving window slice
  • add all pop* multidimensional range primitives
  • add all drop* multidimensional range primitives
  • add dropToNCube
  • assumeSameStructure abstraction
  • indexSlice
  • add opIndexAssign for D arrays
  • add opIndexOpAssign for D arrays
  • Add examples:
    • multidiagonal iteration
    • iteration over diagonal blocks
    • Image processing
    • from numpy vander
@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2015

@burner

This comment has been minimized.

Copy link
Member

commented Jun 9, 2015

put Example 0 in the unittests

the is also some spacing and brace placement issues

@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2015

I'd love to get stuck in to this immediately, but I'm completely swamped with other work.

I don't think we should be targeting phobos so quickly. Whatever the design, it needs to get some proper use before it gets set in stone. I would love to have it as the heart of https://github.com/DlangScience (see my DConf talk) for a start and then see how well it works out.

I imagine a struct NDArray(R, uint nDims) if(isRandomAccessRange!R) { R r; /*...*/ } that defines the full set of operators for indexing and slicing, iteration by element, along any dimension etc. along with element-wise operations (lazily, via map).

Then a matrix type can be trivially built on top of this.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2015

I completely agree.

@9il 9il changed the title Introduce n-dimensional slice-shell [not ready, discussion] Introduce n-dimensional slice-shell [testing, discussion] Jun 13, 2015

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2015

First release http://code.dlang.org/packages/dip80-ndslice.
See also examples at PR's description.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2015

There is problem to build Matrix on top of NDArray/Slice:
Because generalised slicing NDArray requires N strides, but BLASMatrix only 1 (stride between elements in row always equals 1). 2DArray/Slice!2 is generalisation of BLASMatrix that requires runtime check of strides before calling C BLAS functions. We need std.container.matrix, thought.

@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2015

Why can’t you get away with N-1 strides? In terms of the underlying range, one of the N strides is always 1, no?

On 13 Jun 2015, at 20:50, Ilya Yaroshenko notifications@github.com wrote:

There is problem to build Matrix on top of NDArray/Slice:
Because generalised slicing NDArray requires N strides, but BLASMatrix only 1 (stride between elements in row always equals 1). 2DArray/Slice!2 is generalisation of BLASMatrix that requires runtime check of strides before calling C BLAS functions. We need std.container.matrix, thought.


Reply to this email directly or view it on GitHub #3397 (comment).

@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2015

E.g. NxN has 2 strides, (1,N), but you only need store one of them, (N). The transpose of NxN has two strides, (N,1), but again you only need one plus a flag to say "this is transposed".

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2015

In unittest Properties and methods, subsection stride property:

    auto tensor = 100.iota.array.sliced(3, 4, 5);

    assert(tensor.stride   == 20);
    assert(tensor.stride!0 == 20);
    assert(tensor.stride!1 ==  5);
    assert(tensor.stride!2 ==  1);

    //`matrix` can be casted to BLASMatrix
    // strides: (1, 5)
    auto matrix = tensor.back;
    assert(matrix.stride   ==  5);
    assert(matrix.stride!1 ==  1);

    //Runtime error if casting `matrix` to BLASMatrix:
    // strides: (20, 5)
    matrix = tensor.back!2;
    assert(matrix.stride   == 20);
    assert(matrix.stride!1 ==  5);
@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2015

Ah, yes, I see now, my mistake.

@9il 9il changed the title Introduce n-dimensional slice-shell [testing, discussion] n-dimensional slice-shell [ready for comments] Jun 15, 2015

@TurkeyMan

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

This is awesome.
I would like a way to iterate a matrix in a foreach loop, that visits each element at a time, rather than entire rows at once.
Perhaps there is some template that implements multi-dimensional iteration logic? matrix.elementsByRow or matrix.elementsByColumn... you get the idea, except it obviously doesn't map to n-dimensions. I wonder if this sort of API would be possible/convenient for n-dimensions?

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

I would like a way to iterate a matrix in a foreach loop, that visits each element at a time, rather than entire rows at once.

I can add opApply. However delegates can be slow. Other option is deepEach property like each from std.algorithm. What do you think would be better?

Perhaps there is some template that implements multi-dimensional iteration logic? matrix.elementsByRow or matrix.elementsByColumn... you get the idea, except it obviously doesn't map to n-dimensions. I wonder if this sort of API would be possible/convenient for n-dimensions?

The order of dimensions can be changed with generalized transpose (last 2 examples).

    auto t0 = 1000.iota.sliced(3, 4, 5);
    auto t1 = t0.transposed!(2, 0, 1); //CTFE - recommended
    auto t2 = t0.transposed (2, 0, 1); //Runtime
    assert(t0[1, 2, 3] == t1[3, 1, 2]);
    assert(t0[1, 2, 3] == t2[3, 1, 2]);
    static assert(is(typeof(t0) == typeof(t1)));
    static assert(is(typeof(t0) == typeof(t2)));
@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

@9il I think what @TurkeyMan wants is a byElement method. Essentially it's just a byRow.joiner (generalised for n-dims obviously), but joiner doesn't know about rectangular-ness so you sadly only get a forward range.

@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

Instead of offering length at all, I would prefer shape that returns a static array of the dimensions, like numpys ndarray.

All the pointer specialisation is just premature optimisation that obscures the code.

Lazy matrix operations could really take this work to the next level. The bare-bones of the design I had come up with is as follows:

a = b; //shallow copy
a[] = b; //deep copy

auto c = <op>a;
//auto c = Slice!(N, typeof(a._range.map!"<op>a"))(a.lengths, a.strides, a._range.map!"<op>a");

auto d = c <op> b;
//auto d = Slice!(N, typeof(zip(c, b).map!"a[0]<op>a[1]"))(a.lengths, a.strides, zip(c, b).map!"a[0]<op>a[1]");

a <op>= b; //error
a[] <op>= b;
//(<op>b).byElement.copy(a.byElement);

The comments are just for showing the semantics, obviously they could be implemented in a more specialised way.

Also, any operands could be of ElementType!Range as well, e.g. adding a number to every element.

The reason why this is so great, is that it saves hugely on temporaries. Temporary allocations and copies are the bane of my life when working with traditional array-based scientific tools like numpy and matlab. This is very in keeping with Walter's "don't allocate" message.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

@9il I think what @TurkeyMan wants is a byElement method. Essentially it's just a byRow.joiner (generalised for n-dims obviously), but joiner doesn't know about rectangular-ness so you sadly only get a forward range.

OK, we can add both:

  1. Fast byElem (along side with trsansposed and slices we can do anything)
foreach(elem; tensor.byElement) {...}
  1. Slow opApply, but with indexes.
foreach(i, j, k, elem; tensor3D) {...}
@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

Slow opApply, but with indexes.

Why not implement an n-dimensional version of std.range.enumerate and then you can have tensor.byElement.enumerate and everyone wins, no?

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

Slow opApply, but with indexes.

Why not implement an n-dimensional version of std.range.enumerate and then you can have tensor.byElement.enumerate and everyone wins, no?

tensor.byElement.enumerate would not work because lengths is not defined for enumerate.
But Tuple usage is very good! Thank you for this note.
Looks like we need three options
_1. byElement

foreach(elem; tensor.byElement) {...}
foreach(ref elem; tensor.byElement) {...}

_2. byIndexedElement

foreach(i, j, k, elem; tensor3D.byIndexedElement) {...}

_3. opApply for foreach with ref

foreach(i, j, k, ref elem; tensor3D) {...}

or this...

foreach(i, j, k, pointer; tensor3D.byIndexedPointer) {...}
@TurkeyMan

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

I'll just state my use cases, and the most appropriate solution may follow.
I can imagine I will use this mostly to deal with images, and matrices.
In image processing, you want to pass over all pixels. I frequently want to perform some operation akin to a pixel shader, in which case the order is not particularly important, just that the loop must visit each pixel individually. That said, some control over the order is desirable in some cases, so iterating pixels by-row or by-column for images may be useful (effectively a transpose!).
You have obviously considered matrices fairly comprehensively; transpose is the only critical operation there. When considering transpose though, I wonder if other axis mirrors are also useful? Images rarely want to transpose, but they do want to mirror axiis, and they also want to rotate... if you're declaring transpose, which is effectively a tweaked iteration pattern, then perhaps mirrors and rotations should work in the same way?

I think 1, 2, and 3 are all useful. You say opApply is slow above? Why is that?

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

Instead of offering length at all, I would prefer shape that returns a static array of the dimensions, like numpys ndarray.

length!d looks pretty along side with front!d, popBack!d, empty!d and other stuff.
But shape property can be added as well:

TypeTuple!(size_t[N], "lengths", size_t[N], "strides") shape() @property @safe pure nothrow @nogc

All the pointer specialisation is just premature optimisation that obscures the code.

Lazy matrix operations could really take this work to the next level. The bare-bones of the design I had come up with is as follows:

a = b; //shallow copy

Already works.

a[] = b; //deep copy

Already works.

auto c = op>a;
//auto c = Slice!(N, typeof(a._range.map!"a"))(a.lengths, a.strides, a._range.map!"a");

It is very ambiguous syntax.
Current state (with shape) for ranges (not for Sliced(size_t N, T*)):

auto c = a.range.map!"fun>".sliced(a.shape.lengths);

What about slicedMap template?

auto d = c op> b;
//auto d = Slice!(N, typeof(zip(c, b).map!"a[0]op>a[1]"))(a.lengths, a.strides, zip(c, b).map!"a[0]op>a[1]");

ditto
slicedZip?

a op>= b; //error

Already works (error).

a[] op>= b;
//(op>b).byElement.copy(a.byElement);

Already works. (Implemented in more specialised way)

The comments are just for showing the semantics, obviously they could be implemented in a more specialised way.

Also, any operands could be of ElementType!Range as well, e.g. adding a number to every element.

I don't understand this bit.

The reason why this is so great, is that it saves hugely on temporaries. Temporary allocations and copies are the bane of my life when working with traditional array-based scientific tools like numpy and matlab. This is very in keeping with Walter's "don't allocate" message.

Agreed. However your syntax looks like "I allocate" instead of "I don't allocate".

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

EDIT: auto sliced(...)(Range, Tuple(...) shape) should be added, thought

auto c = a.range.map!"<fun>".sliced(a.shape);
@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

I think 1, 2, and 3 are all useful. You say opApply is slow above? Why is that?

Because it calls delegate. For example: if you want to set up some integer slice to simple funciton from indexes it would be approximately two times slower then naive chain of foreach loops.

What do you think about byIndexedPointer?

foreach(i, j, k, pointer; tensor3D.byIndexedPointer)
{
    pointer* = i*j+k;
}
@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

R.e. slicedMap, slicedZip, ambiguous syntax:

It isn't super-explicit, but it is very usable.

Consider this sort of function:

auto imageAdjust(S0, S1)(S s, float gamma, float preOffset, float postOffset, S1 mask)
{
    return (postOffset + (s + preOffset)^^gamma) & mask;
}

The maths is directly visible but no allocations needed (compared to the same in numpy, where you'd be allocating like crazy).

Compare to the same without the syntax sugar:

auto imageAdjust(S0, S1)(S s, float gamma, float preOffset, float postOffset, S1 mask)
{
    return slicedZip(s, mask).map!(x => (postOffset + (x[0] + preOffset)^^gamma) & x[1]);
}

And then consider how it scales are more Slices are involved. Also, note that the lambda in the non-sugar version would have to be allocated as a closure.

I'm not sure how I would sell D to a normal data scientist if I told them that they can't write simple mathematical expressions on multidimensional arrays without using template lambda functions, declaring all the arrays involved up-front and then referring to them in the expression by indexing a Tuple.

The other big win of supporting these basic algebraic operations is that it allows some code to be written once and support everything from scalars through to 1000-dim arrays. The imageAdjust function works just as well if you pass it two Slices or two floats, or even (if D's simd/arrayOp support was better), float4 or float[32].

@TurkeyMan

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

What's the advantage of the pointer api vs ref? I don't think foreach syntax and pointers should mingle.

@TurkeyMan

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

Another thought; Walter dropped the ideas of some lowering for array syntax to range functions by the compiler in his talk... consider how those ideas interact with multi-dimensional arrays?

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

What's the advantage of the pointer api vs ref? I don't think foreach syntax and pointers should mingle.

Nothing in terms of syntax. Only speed (Tuple vs delegates).

OK, lets droop byIndexedPointer. We can use foreach chain for speed or opApply.
It is fine because foreach without indexes can be both ref and not (case 1).

@9il

This comment has been minimized.

Copy link
Member Author

commented Jun 15, 2015

Another thought; Walter dropped the ideas of some lowering for array syntax to range functions by the compiler... consider how those ideas interact with multi-dimensional arrays?

@TurkeyMan Please describe what do you mean.

@John-Colvin

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2015

Just a heads-up that I've edited my previous post to clarify some things, I'll delete this message shortly.

@9il 9il changed the title std.experimental.ndslice [voting] std.experimental.ndslice [ready to merge] Dec 28, 2015

@9il

This comment has been minimized.

Copy link
Member Author

commented Dec 28, 2015

Voting is over with the final result of 12-0. If there are no major issues found by the core maintainers then this can be merged!

@JackStouffer Thank you for review management!

@9il

This comment has been minimized.

Copy link
Member Author

commented Dec 28, 2015

I have removed compiler version check. It can be merged IMO.


///ditto
Slice!(N, Range) transposed(size_t N, Range)(auto ref Slice!(N, Range) slice, size_t dimension)
in {

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 28, 2015

Member

Brace style

mixin (DimensionRTError);
}
body {
size_t[1] permutation = void;

This comment has been minimized.

Copy link
@JackStouffer

JackStouffer Dec 28, 2015

Member

Why not just size_t[1] permutation; or size_t[1] permutation = [dimension];

This comment has been minimized.

Copy link
@9il

9il Dec 28, 2015

Author Member

This is common style in this package.

This comment has been minimized.

Copy link
@9il

9il Dec 28, 2015

Author Member

Furthermore this construction look clear in terms of optimisation.

@9il

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2015

@burner By taking into account comment by @andralex and my comment could you please allow to count your vote as simple Yes? Please ^___^

BTW: Have a Great New Year!

@dnadlinger

This comment has been minimized.

Copy link
Member

commented Dec 31, 2015

I've been largely absent from Phobos development lately because of other commitments. The current PR contains at least one object file, though, which should be removed completely from history.

@9il

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2015

I've been largely absent from Phobos development lately because of other commitments. The current PR contains at least one object file, though, which should be removed completely from history.

Thanks! Fixed and rebased

@9il

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2015

Merge failed for DAutoTest. In the same time auto-tester works well .
ping @CyberShadow

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 1, 2016

DMD 2.070 is going to be branched 1st Jan. @andralex @MartinNowak @klickverbot @kyllingstad @burner please make a decision about ndslice package for 2.070 release ASAP.

Notes:

  1. Documentation: link
  2. Final votes
  • 11Yes
  • 1 Conditional Yes by @burner (because explanation style in documentation)
  • 0 No

Thanks!

@CyberShadow

This comment has been minimized.

Copy link
Member

commented Jan 1, 2016

Merge failed for DAutoTest. In the same time auto-tester works well .

Fixed now (and one step closer to fixing this permanently), thanks for letting me know. Happy new year.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 1, 2016

@CyberShadow Thanks! Happy new year)

@burner

This comment has been minimized.

Copy link
Member

commented Jan 1, 2016

I still think that the docs need improvements

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 1, 2016

I still think that the docs need improvements

OK, if this is hard constraint, then a decision maker from Core team can close this PR.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 1, 2016

@CyberShadow DAutoTest failed again .

@CyberShadow

This comment has been minimized.

Copy link
Member

commented Jan 1, 2016

I still think that the docs need improvements

I am not much involved in this project but I think docs can be imploved post release. As long as the design is sound and backwards-incompatible changes are unlikely, it could be merged.

@CyberShadow DAutoTest failed again .

Fixed

initial commit
fix Slice

change concept

implement `sliced`

change structure

remove comment

slice constructors

add 1D case

fix opSlice 1D

implement ND, part1

update to modern syntax

generic opIndex

fix protection

cleanup

fix

cleanup

style fix

rename

update

update

implement transpose

unittest

unittest for properties and methods

more examples & fixes

move code

minor fix

add header

update make fils

update make files

dmd bug workaround

fix module name

update imports

style fix

fix asserts

remove createRefCountedSlice

add shape property

rework `shape`, add `structrure`

ndarray

swapped

transposed

opCast

remove save calls

everted

make Slice(size_t N0, Slice(size_t N1, ...)) virtual

huge rework

remove space

move code

fix style

add packEverted

relax constraints

add black comments

remove spaces

fix macros

fix doc

fix docs

update docs

reduce template bloat

optimize and fix for arrays.

update docs

remove space

update docs

update docs

update link

update doc

fix constructor

add toString

fix `save`

add `reversed` method

fix constraints for `reversed`

optimisation in unittests

add `strided`

implement `reversed` for ranges (no arrays)

reduce constraints

cleanup

add Iota for static foreach

remove string representation

DMD 2.067 support

fix style

fix opIndexUnary constraints

move byElement

add shift property

add CTFE-able strided

huge style fix

add macros

update `sliced`

move private code

update docs

update allIndexesReversed, renamed to allReversed

update docs

update docs

fix macros

fix posix.mak

update posix.mak

update docs and strided

move code

update sliced constraints

update `sliced` docs

move code

remove whitespaces

add static assert to ndslice

add one more opIndexUnary

update createSlice docs

update ndarray

update docs for Transpose operators

update docs and fix bugs

add pragma inline

fix docs

update docs

update docs

add inline pragma

ditto

replace errors with asserts

update docs

update doc style

update docs

remove comes in docs

update docs

remove whitespaces

update docs

update docs

update comment

update test

update docs

fix typo

fix docs

review

fix transposed description

change doc tables

remove unused RepeatTypeTuple

remove function attributes for templates

make constructor private

make N and Range private

make elementsCount public

fix createSlice params naming

add assert description to sliced

[big] fix range primitives wrong asserts and update documentation

regroup primitives

minor docs fix

minor docs fix

fix typo

fix Slice constraints

add indexing using static arrays

make byElement a random access range

fix by random access primitives for ByElement

update unittest

fix random access primitives for ByElement

remove trailing space

implement slicing for ByElement

make ByElement more readable

update docs for subspace operators

remove one See_also

revert last commit

update docs

add descriptions to asserts

add more examples

minor doc update

add example with allocators

add makeSlice for allocators

update docs table

add range checks

add more constructors

Add description to asserts.

add checks for const/immutable

ditto

update to DMD 2.069

minor fixes

add elements property

make makeSlice an unittest

remove space

update docs

remove space

update docs

update doc

fix makeSlice example

fix strided

make strided template variadic

add Guide for Slice/Matrix/BLAS contributers

remove unused import

add better error messages

update guide

update docs

remove space

[minor] fix docs

minor error messages update

minor doc fix

rename package

split package

update posix.mak

update win*.mak

ditto

fix posix mak

update *mak.

update docs

update man files

minor doc update

update module headers

fix opCast

rename pop*N to pop*Exactly

remove primitives

add popFrontN popBackN to Slice

[minor] update docs

add package primitives to Slice

update operators prototypes

add drop* primitives

[minor] update docs

update docs

remove spaces

remove allocators

minor doc fix

[minor] update docs

add dropToNCube

add diagonal

add return type for template for better documentation

move pack* to iterators

rm allocators

[minor] doc update

add support of packed slice for diagonal

update example for diagonal

add blocks

rename packed and unpacked to pack and unpack

update docs

renaming

[minor] docs update

ditto

minor style update

rm old files

[minor] update docs

update docs

ditto

[minor] update docs

add rotated

[minor] update docs

add byElementInStandardSimplex

add windows

remove space

remove structure

update docs

add spaces

add reshape

rename packEverted -> evertPack

remove spaces

fix ReshapeException

minor doc fix

update windows/blocks template params

fix pack windows/blocks

add index @Property

remove spaces

minor doc fix

add Slice tuples

remove spaces

update docs

update docs and rename dropToHypercube

update docs

minor doc update

fix links

remove version OS constraints for allocators

assumeSameStructure

fix

minor doc fix

minor doc update

after review

minor style fix

fix Elaborate Assign

add index slice

fix NSeq

fix bug with diagonal

fix sliced slice

add main example

update docs

translation

fix comment

fix style

remove spaces

update style

fix link

Vandermonde matrix

remove space

move example

remove `opCast`

add opEquals for arrays

update opIndex(Op)Assign

update docs

update docs

fix style

update docs (russian will be translated)

update tests

fix doc style

ditto

ditto

update docs

update docs

update docs

update docs

update unittests

update docs

ditto

ditto

ditto

[major] doc update

update docs

update docs

fix voting conditions (docs, style)

minor doc update

fix style

add unittest for `map`

ditto

fix string mixins

add Params and Returns

ditto

add headers

add descriptions

Fix m32 mode in example

Minor description fix

fix spaces

ditto

Add Internal Binary Representation

ditto

ditto

ditto

ditto

ditto

ditto

ditto

add description for binary representation

ditto

minor style fix

ditto

ditto

ditto

ditto

ditto

ditto

inlining

remove compiler version check

fix braces

fix docs

add Quick_Start

Fix English

Add two examples

fix style

remove object file

minor doc update

ditto

remove spaces

fix indexing & add unittests

ditto
@MartinNowak

This comment has been minimized.

Copy link
Member

commented Jan 2, 2016

I still think that the docs need improvements

I think @9il will have a hard time to improve the docs on it's own, and it's something that's better done collectively while working the module from std.experimental to std.

@MartinNowak

This comment has been minimized.

Copy link
Member

commented Jan 2, 2016

That said the documentation doesn't need a few improvements but a complete restructure. There is no clear introduction/overview, just the table for the submodules. When I try to understand the example (which seems a bit too hard for an intro) I'm already stuck trying to understand what pack does.
And it's documentation isn't too helpful.
http://dtest.thecybershadow.net/artifact/website-8566711a7833f9dcdf044cac5c48bfe666251245-1b1f44a3f35545b790eac434df7d7835/web/phobos-prerelease/std_experimental_ndslice_selection.html#pack

@MartinNowak MartinNowak added this to the 2.070.0 milestone Jan 2, 2016

@MartinNowak

This comment has been minimized.

Copy link
Member

commented Jan 2, 2016

Auto-merge toggled on

@MartinNowak

This comment has been minimized.

Copy link
Member

commented Jan 2, 2016

Let's move on with this, there has hardly been a module w/ more support by the community, and as there are many users (and already articles in preparation), I'm pretty confident, we can get the documentations in shape soon.

MartinNowak added a commit that referenced this pull request Jan 2, 2016

Merge pull request #3397 from 9il/slice
std.experimental.ndslice [ready to merge]

@MartinNowak MartinNowak merged commit cda90ee into dlang:master Jan 2, 2016

2 checks passed

CyberShadow/DAutoTest Documentation OK (135417 additions, 8 deletions)
Details
auto-tester Pass: 10
Details
@jacob-carlborg

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2016

That's a really crappy commit message (9il@54a6d72) 😞.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 2, 2016

Whoohoooo!
Many thanks to all reviewers and @JackStouffer, ndslice review manager.
@John-Colvin I have mentioned you in Acknowledgment section in package description.

@9il

This comment has been minimized.

Copy link
Member Author

commented Jan 2, 2016

#3896 #3897 upgrade win*.mak & uncomment unittest with dummyranges

@9il 9il added the ndslice label Apr 17, 2016

@JackStouffer

This comment has been minimized.

Copy link
Member

commented Apr 19, 2016

According to http://digger.k3.1azy.net/trend/, this caused a 85KiB increase in the hello world binary!? Is DMD really including symbols from this when it's never imported?

@wilzbach

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

According to http://digger.k3.1azy.net/trend/, this caused a 85KiB increase in the hello world binary!?

@JackStouffer wasn't it dlang/druntime#1453 ?

edit: I had to zoom in many times and then found: dlang/dmd#5324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.