ndarray: getindex/setindex! linear indexing #294
Conversation
iblislin
commented
Oct 9, 2017
iblislin
commented
```julia x = mx.zeros(2, 5) x[5] = 42 ```
Codecov Report
@@ Coverage Diff @@
## master #294 +/- ##
=========================================
Coverage ? 69.43%
=========================================
Files ? 25
Lines ? 1924
Branches ? 0
=========================================
Hits ? 1336
Misses ? 588
Partials ? 0
Continue to review full report at Codecov.
|
| idx %= offset | ||
| end | ||
|
|
||
| _at(handle, idx) |> MX_NDArrayHandle |> x -> NDArray(x, arr.writable) |
There was a problem hiding this comment.
It is kinda weird to return a one element array and also not common in Julia. The problem is if we convert it to a Julia value at this point we impose a synchronisation barrier.
There was a problem hiding this comment.
I agree with the weirdness, Python provide an API nd.asscalar() for conducting reading value.
I think in Julia, we can have first for reading it.
looks like this
x[42] |> first # do synchronisation and get the valueand keep x[42] returning an NDArray
There was a problem hiding this comment.
I can send a PR for first api. If you agree with that design
There was a problem hiding this comment.
+1 for the first design. I think we should make it explicit for users that an NDArray is not some transparent array, and it could potentially be on GPU and could potentially be slow to do indexing into every single element of it.
BTW: I feel I'm pretty out-dated about the latest Julian things when I see the |> and ∘ operators. :D
There was a problem hiding this comment.
first implemented @ 62bd213
| @@ -26,6 +26,26 @@ | |||
| 2.0 4.0 | |||
| ``` | |||
|
|
|||
| * `NDArray` `getindex`/`setindex!` linear indexing support and `first` for extracting scalar value. (#TBD) | |||
There was a problem hiding this comment.
(at first I wrote this patch, the issue number was still unknown)
I will send a PR to sort them out for releasing.
|
Thanks! LGTM. Can you resolve the conflicts in NEWS.md? |
|
resolved |
