arrayfire / arrayfire-rust

Rust wrapper for ArrayFire
BSD 3-Clause "New" or "Revised" License
815 stars 57 forks source link

Feature request: Serialization and deserialization using serde #221

Closed JonathanWoollett-Light closed 4 years ago

JonathanWoollett-Light commented 4 years ago

An implementation similar to how ndarray implements serde for serialization and deserialization would be a useful feature to see.

9prady9 commented 4 years ago

I don't know how ndarray is implemented but Array from arrayfire-rust is a RAII style construct.

I think this can be implemented by calling Array::host method to fetch data from GPU memory to host memory - at which point this regular array can be serialized using serde as usually.

Having said that, above logic can be easily implemented at user level too since I wouldn't imagine if we would do anything different from the above.

@umar456 @syurkevi any suggestions ?

My only concern is, is it worth it to add a whole new runtime crate dependency just for serialization when it can be very easily implemented using function Array::host ? Perhaps, serialization feature as separate crate makes sense :thinking:

ethanhs commented 4 years ago

@9prady9 serde is the defacto standard for serialization and deserialization in Rust. It would be great if there was a feature for ArrayFire Rust that would enable serde serialization. Otherwise I'd have to implement it myself, which is a) more complicated and b) re-creates a lot of work that belongs in this repo IMO.

ethanhs commented 4 years ago

I came across this: https://github.com/shibii/arrayfire_serde but it is for an older version. I forked it here: https://github.com/ethanhs/arrayfire_serde

9prady9 commented 4 years ago

@ethanhs What all types would require this support in your opinion ? I haven't looked at the repository you shared yet, but it does talk about sedre support for all types of ArrayFire.

I have removed the discussion tag for this issue. We can work on this feature, but unfortunately I can't give an exact ETA on this. You are welcome to contribute the change if you are interested working on it.

ethanhs commented 4 years ago

@9prady9 The crate I forked implements it for Array, Dim4, and DType.

9prady9 commented 4 years ago

@JonathanWoollett-Light @ethanhs https://github.com/arrayfire/arrayfire-rust/pull/250 is making good progress. Feel free to test it out. You would have to enable afserde feature to build the serde support while building the project. You can do so via the following command

cargo test --all-features --features=afserde -- --nocapture --test serde

https://arrayfire.org/arrayfire-rust/arrayfire/index.html#structs That's the list of structs. I have listed the structs that need serde support to the best of my knowledge in the above mentioned PR description. If any other struct needs it, please do let me know.