search

mratsim / Arraymancer

A fast, ergonomic and portable tensor library in Nim with a deep learning focus for CPU, GPU and embedded devices via OpenMP, Cuda and OpenCL backends
https://mratsim.github.io/Arraymancer/
Apache License 2.0
1.33k stars 95 forks source link

Add slice, reshape, axis views for no-copy tensor manipulation #46

Closed mratsim closed 6 years ago

mratsim commented 6 years ago

shallowSlice and shallowReshape are added. Tests pending.

shallowAxis(yields reference) and maxis (yields mutable reference, similar to items) are planned but will probably be blocked by https://github.com/nim-lang/Nim/issues/5887 (shallowCopy in iterators yields full copies).

Discussion:

  1. Add unsafeShallowSlice which can take "let" parameter (and not var) and writing fast libraries that uses Arraymancer. cc @edubart. Relevant discussion about shallow vs unsafeShallow in https://github.com/nim-lang/Nim/issues/644 and https://github.com/nim-lang/Nim/issues/3377

  2. RFC in Nim bug tracker about slicing Views - https://github.com/nim-lang/Nim/issues/5753. Should we have a separate view type and have proc generics over Tensor and TensorView

My opinion:

  1. It is important to provide sane defaults but allow higher performance when needed/possible. unsafeShallow while long is good because easily grep-able and safe code is by default/easier to write ( '=' for copy, shallowSlice for var shallow, unsafeShallowSlice for let shallow). Shallow copy by default is also a source of (bad) surprise for Numpy users. image Another explanation from the Matrix Template Library on the issue of shallowCopy for numerical software. When https://github.com/nim-lang/Nim/issues/6348 is solved, Arraymancer will implement "move" optimization so that tensors with refcount of 1 will get shallow-copied on assignment using this. Also it is possible to rely on Nim GC to efficiently get refcount with proc getRefcount.

  2. Implementing Tensor and TensorView, CudaTensor and CudaTensorView, etc seems too cumbersome to use and maintain. Having shallow, unsafeShallow and getRefcount should already be explicit and flexible enough.

edubart commented 6 years ago
  1. Implementing unsafeShallowCopy will be useful, there are many cases that user may want to work with just an slice of a tensor. For example multiplying a slice of tensor with a matrix, in this case having an extra copy by value of the slice would have a bad performance and the user might be inside a method where this tensor is a let variable, so in this case unsafeShallowCopy is needed.

I suggest that unsafeShallowCopy could be called just view or if you want to avoid user mistakes unsafeView. View meaning it much more clear in the Tensor/Matrix context. Or other name option would be unsafeSlice.

  1. If things workout for shallow slicing in both usability and performance-wise I don't see the needs for a separate TensorView object at the moment. Unless for user safety reasons, like removing elements assignment procs from the tensor view to prevent misuse of a read only slice view.
mratsim commented 6 years ago

I added unsafeView (for copy and slicing) and unsafeReshape (for contiguous tensors only).

I don't think the issue with shallowCopy in iterators will be closed anytime soon so for now axis shallowCopy will not work. (And I don't think there are critical algo relying on that)

To be reopened when https://github.com/nim-lang/Nim/issues/5887 is closed.