Implement Neg overloaded operator for Array about arrayfire-rust HOT 8 CLOSED

This has been fixed in #94

@Timmmm Can you please elaborate on what you want to know about copying/cloning w.r.t ArrayFire.

from arrayfire-rust.

Like, does cloning an array create a new reference to the same underlying data (shallow copy), or does it make a new array and copy the data across. I assume the latter since that's how it normally works but it might be nice to be explicit for newbies like me.

from arrayfire-rust.

@Timmmm Array.clone() is a shallow copy and it is mentioned in the documentation of Array structure in the Traits implementations section.

from arrayfire-rust.

Hmm you might want to make that more explicit in a more obvious place in the documentation. At the moment the only hint I can see is that .clone() is "Used for incrementing the reference count of Array's native resource". Also doesn't this go against the normal Rust rules? E.g. I can now do this:

let mut p = constant::<f32>(0.0, dims);
let mut p2 = p.clone();
// Now I have two mutable references to the same underlying data, which is generally forbidden.

I might be wrong about this. In any case it is definitely unexpected and worth documenting more. Maybe add something like this:

A multidimensional data container. The Array struct holds a reference to the underlying ArrayFire array. Cloning this Array will not create a new ArrayFire array but will give you two references to the same underlying array. To create a deep copy use copy().

Also the documentation of copy() seems to suggest it does a shallow copy ("Internally, this is handled by reference counting"), but looking at the source it uses af_copy_array() which apparently is a deep copy. I'd suggest changing the copy() documentation to just "Makes a deep copy of the Array" and remove the line about reference counting.

Sorry this is a bit off-topic; I'll make another issue if you like. Also I can do a pull request if you're ok with that wording.

Edit: Now that I think about it, is this handled by everything being copy-on-write? Well even if so it needs to be documented!

from arrayfire-rust.

@Timmmm Thanks for pointing out the mistake in documentation about copy method of Array struct. I have corrected this. I have also added the suggested explanation as part of clone's documentation.

let mut p = constant::<f32>(0.0, dims);
let mut p2 = p.clone();
// Now I have two mutable references to the same underlying data, which is generally forbidden.

Rust uses move semantics and it makes sense to use clone to create a copy of a value if we want to reuse it else where. However, in the case of a struct that is managing native resource such as GPU memory, it is not just copy of few bits or bytes. Copying data from one location to another is expensive operation which should be delayed(copy-on-write) or avoided(using reference counting) depending on the context in which the said Array is being utilised. Hence, clone increments only reference count and the user has to explicitly ask for deep copy if he wants one using copy method.

Having said that, i have made the corrections to documentation based on your feedback.
Thank you once again!

from arrayfire-rust.

Isn't that why Rc exists? On the other hand maybe ArrayFire implements its own thread safety so having two mutable references to the same array is fine? Well it's not a big issue as long as the documentation is clear - thanks for updating it!

from arrayfire-rust.

@9prady9 @Timmmm To give some background, arrayfire has copy on write semantics internally. This sometimes involves creating shallow copies as long as the data is being read, but create a new copy when data is being written to the array.

The underlying logic in the arrayfire lib uses shared_ptr for the gpu buffers that is shared between two arrays. There is no reason to reimplement the same logic in the rust wrapper.

On a related note, arrayfire is not thread safe yet. @9prady9 is working on it. You can see the progress being made here: arrayfire/arrayfire#1706

from arrayfire-rust.

Rc doesn't implement Send trait and thus cannot be sent between threads, so it doesn't provide thread safety access to the data. Arc on the other hand does what you are saying. However, Copy-On-Write takes care of having two mutable references situation you have mentioned.

On another note, we are currently working on adding thread safety feature to the upstream(ArrayFire) library on the whole. We made sure all ArrayFire's upstream libraries resource management and the internal resource management is thread safe. However, we don't intend on doing any synchronisation under-the-hood for af_array resource handle. Access to af_array is thread-safe only when two different threads are reading from it. The developer has to synchronise the writes themselves. In case of Rust wrapper, I believe this works in favour of the thread-safety due to Rust's move semantics compared to C++ or other similar languages.

from arrayfire-rust.