DlangScience / scid

Scientific library for the D programming language
Boost Software License 1.0
90 stars 31 forks source link

modern D array indexing/slicing #24

Open John-Colvin opened 9 years ago

John-Colvin commented 9 years ago

A couple of notes:

MatrixView has opIndex return by ref in order to allow array[i,j] op= v;. The should be done with opIndexOpAssign instead.

With the new extended semantics of opIndex and opSlice, a proper NDArray type can be implemented, with multi-dimensional slicing.

http://dlang.org/operatoroverloading.html

kyllingstad commented 9 years ago

MatrixView has opIndex return by ref in order to allow array[i,j] op= v;. The should be done with opIndexOpAssign instead.

Good point! opIndexOpAssign didn't exist when this code was written. That would take care of the issue with the zero elements in a triangular matrix being modifiable. I'll fix it.

With the new extended semantics of opIndex and opSlice, a proper NDArray type can be implemented, with multi-dimensional slicing.

I know, and agree that would be nice to have. However, if I were to dedicate some time to doing something with the new slicing features, I think I would prioritise implementing submatrix slicing first, before going all out and creating a full-fledged N-dimensional array type. (Also, that seems general enough to be in Phobos, if you ask me.)

9il commented 9 years ago

You can copy-past if it is needed https://github.com/9il/simple_matrix/blob/master/source/simple_matrix.d#L432

9il commented 9 years ago

Code example from corresponding discussion:

size_t anyNumber;
auto ar = new int[3 * 8 * 9 + anyNumber];
auto slice = Slice[0..3, 4..8, 1..9];
assert(ar.canBeSlicedWith(slice)); //checks that ar.length <= 3 * 8 * 9

auto tensor = ar.sliced(slice);
tensor[0, 1, 2] = 4;

auto matrix = tensor[0..$, 1, 0..$];
assert(matrix[0, 2] == 4);
assert(&matrix[0, 2] is &tensor[0, 1, 2]);
9il commented 9 years ago

Slice implementation is very simple:

struct Slice
{
static @safe pure nothrow @nogc:

    size_t[2][Args.length] opIndex(Args...)(Args args)
        if (allSatisfy!(isSize_t2, Args))
    body {
        return [args];
    }

    size_t[2] opSlice(size_t p)(size_t a, size_t b)
    in {
        assert(a <= b);
    }
    body {
        return [a, b];
    }

    @disable this();

private:

    enum isSize_t2(T) = is(T == size_t[2]);

}

///
unittest {
    auto slice = Slice[0..1, 3..4, 2..3];
    static assert(is(typeof(slice) == size_t[2][3]));
    assert(slice == [[0, 1], [3, 4], [2, 3]]);
}
9il commented 9 years ago

Phobos PR https://github.com/D-Programming-Language/phobos/pull/3397

kyllingstad commented 9 years ago

Nice! If this goes through in Phobos, I guess MatrixView can be removed from SciD altogether? Your Slice seems to do the same job.

9il commented 9 years ago

Yes. Current (example in Phobos PR) concept is simplified, but logic is the same.