Skip to content

Typed Arrays? #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost opened this issue Jan 25, 2017 · 18 comments
Closed

Typed Arrays? #98

ghost opened this issue Jan 25, 2017 · 18 comments
Assignees

Comments

@ghost
Copy link

ghost commented Jan 25, 2017

Hi folks,

Thanks for such an awesome official binding, this has opened up a lot of possibilities for Rust and Machine Learning experimentation for me, on my AMD card.

One head-scratching thing for me though, is why Arrays aren't typed in the Rust API? The Arrays are constructed with a typed slice/array, so the information is there at compile time. And, there are methods to get type, so type is clearly stored/available at runtime.

But, there's a big difference between doing a runtime match over the results of an array's get_type method, and building a true generic dispatch. For example, here's what I have right now for a left-rotate:

fn gpu_rotl(x: &af::Array, n: u8) -> af::Array {
    let t = x.get_type();
    let dsize : u8 = match t {
        af::DType::U8 => 8,
        af::DType::U16 => 16,
        af::DType::U32 => 32,
        af::DType::U64 => 64,
        _ => panic!("Array passed to rotl is not of unsigned integer type: {:?}", t),
    };
    let ls = af::shiftl(x, &n, false);
    let rsn = dsize - n;
    let rs = af::shiftr(x, &rsn, false);
    af::bitor(&ls, &rs)
}

AFAIK, the above will result in a runtime match. But, if it were typed appropriately, it might be possible to get a compiled static-dispatch. I imagine that would be better for ArrayFire's JIT, because there wouldn't be a context switch from the GPU operations to the CPU to decide the next action, there would just be a hard-compiled set of operations.

At the very least, appropriate typing would help catch bugs. But I think this might be a performance issue, too?

@9prady9
Copy link
Member

9prady9 commented Jan 26, 2017

Thank you for your kind words.

Not sure, how an Array generic over type T would help in situation as above ? Can you please show some how the rotl code might look like if Array is typed.

It is true that the type information is stored, but not at rust level. Rust Array is resource managing wrapper for af_array (C handle).

@ghost
Copy link
Author

ghost commented Feb 1, 2017

Perhaps some broken code would help more, here.

I've tried to re-do this blog post in Rust using ArrayFire, as a learning exercise.

I keep getting segfaults or obscure bugs, some examples listed below, which hint at type unsafety in the Rust bindings.

My code looks like this:

pub fn sigmoid_prime(a: &af::Array)->af::Array {
    // NB: I have to specify 1f32 instead of something like `1 as a.T`
    // (not even sure that's Valid rust, but I think you get the idea)
    af::sigmoid(a) * ((af::sigmoid(a) * -1f32) + 1f32)
}

fn main() {
    let x_dims = af::Dim4::new(&[3, 4, 1, 1]);
    let y_dims = af::Dim4::new(&[4, 1, 1, 1]);
    let x = af::Array::new(&[
        0f32, 0f32, 1f32,
        0f32, 1f32, 1f32,
        1f32, 0f32, 1f32,
        1f32, 1f32, 1f32
        ] as &[f32; 12], x_dims);
    let y = af::Array::new(&[0, 1, 1, 0], y_dims);
    af_print!("x", x);
    af_print!("y", y);
    // NOTE: I can't re-use randomengine, or randomenginetype?
    // Also, if I use Mersenne option, this breaks at runtime!
    let mut syn0 = af::random_normal::<f64>(af::Dim4::new(&[3, 4, 1, 1]),
                                        af::RandomEngine::new(af::RandomEngineType::PHILOX_4X32_10, Some(1)));
    let mut syn1 = af::random_normal::<f64>(af::Dim4::new(&[4, 1, 1, 1]),
                                        af::RandomEngine::new(af::RandomEngineType::PHILOX_4X32_10, Some(2)));
    af_print!("syn0", syn0);
    af_print!("syn1", syn1);
    for j in 0..60000 {
        // Feed forward through layers 0, 1, 2
        let l0 = x.clone();
        if j == 0 {println!("Feedforward")}
        if j == 0 {println!("Calculating layer 1 dot prod")}
        // Neither of the below methods work, at runtime. af::dot breaks because of
        // "Function does not support GFOR / batch mode" (But no batch arg?),
        // whereas multiplying with "*" works for l1_dots and breaks on l2_dots with
        // "Size is incorrect".
        let l1_dots = af::dot(&l0, &syn0, af::MatProp::NONE, af::MatProp::NONE);
        //let l1_dots = l0.clone() * syn0.clone();
        if j == 0 {println!("Calculating layer 1 activations")}
        let l1 = af::sigmoid(&l1_dots);
        if j == 0 {println!("Calculating layer 2 dot prod")}
        let l2_dots = af::dot(&l1, &syn1, af::MatProp::NONE, af::MatProp::NONE);
        //let l2_dots = l1.clone() * syn1.clone();
        if j == 0 {println!("Calculating layer 2 activations")}
        let l2 = af::sigmoid(&l2_dots);
        // How much did we miss target?
        if j == 0 {println!("calc l2_err")}
        let l2_error = af::sub(&y, &l2, false);
        if (j % 10000) == 0 {
            let ab_err = af::abs(&l2_error);
            let ab_mean= af::mean(&ab_err, 0);
            af_print!("Error:", ab_mean);
        }
        // In what direction is target value?
        // Were we really sure? If so, don't change too much.
        if j == 0 {println!("calc l2_delta")}
        let l2_delta = l2_error * sigmoid_prime(&l2);
        // How much did each l1 value contribute to the l2 Error
        // (according to the weights)?
        if j == 0 {println!("calc l1_err")}
        let l1_error = af::dot(&l2_delta, &syn1, af::MatProp::NONE, af::MatProp::NONE);
        // In what direction is target l1?
        // Were we really sure? If so, don't change too much.
        if j == 0 {println!("calc l1_delta")}
        let l1_delta = l1_error * sigmoid_prime(&l1);
        // Update weights
        if j == 0 {println!("update weights")}
        let syn1_upd = af::dot(&l1, &l2_delta, af::MatProp::NONE, af::MatProp::NONE);
        syn1 = af::add(&syn1, &syn1_upd, true);
        let syn0_upd = af::dot(&l0, &l1_delta, af::MatProp::NONE, af::MatProp::NONE);
        syn0 = af::add(&syn0, &syn0_upd, true);
    }
    af_print!("Final syn0", syn0);
    af_print!("Final syn1", syn1);
}

The above will compile, but crash at runtime, if:

  • I change the random engine to Mersenne
  • I use af::dot to multiply arrays. I could not find any examples or tests showing correct usage of af::dot.
  • I use the * operator to multiply; it works for one array pair, panics on the other.

I have not gotten past the dot step, so more runtime errors will probably follow further down.

I regularly see runtime crashes where arrays containing different types are passed to af:: functions that operate on two arrays. This is something that type safety should be able to solve pretty simply, if the array objects contained their type in a compile-checkable way.

If my usage of af::dot is incorrect, perhaps that's also something that type-safety would help with.

Thanks

@9prady9
Copy link
Member

9prady9 commented Feb 1, 2017

@cathalgarvey Thanks for the example code and feedback, i will look into it.

As far as the run time crash goes, in the cases such as mismatched error types, arguments etc. they are panics reported by the callee function.

We chose to follow non-rusty (not return Result objects) way to handle errors due to various reasons. You can handle these panics using panic::catch_unwind to catch the panics that occur while calling a function. I understand that there needs to be tutorial sort of thing explaining something like this. We will have something similar up and available soon on the rust documentation or github soon.

@ghost
Copy link
Author

ghost commented Feb 2, 2017 via email

@9prady9
Copy link
Member

9prady9 commented Feb 2, 2017

Target applications of ArrayFire library largely revolve around mathematical equations and/or expressions that can be complex such as the ones in shallow water equation example on github. Expressing such code in rust using idiomatic error management makes it cumbersome for the developer to handle Result objects at end of every other binary/unary operation or function call. For example,

Callback Error Management
Array d = (a/b + a*c)^e; // Expression version
Array g = exp(add(div(a, b), mul(a, c)), e);
Idiomatic Error Management
Array d = (a/b + a*c)^e; 
// Expressions such as the above can't be done in terse
// manner if output of an unary or binary operation is Result 
// object unless the operator traits accept Result objects.

Array g = exp(add(div(a, b).unwrap(), mul(a, c).unwrap()), e);
// Function versions of binary operations either have to accept Result
// objects or each result of function should be unwrapped or handled
// using a match statement.

Note: I created the code stubs to illustrate the idea, they may need some changes to actually compile.

Our take away point from this was that, we had to support two versions(Array's and Result) of functions everywhere to handle all cases of function arguments. This would result in too much code redundancy. The verbosity of code that can be achieved creating two versions of functions can also be achieved using callback based error mechanism all the while keeping the code redundancy to as low as possible. Hence, we chose callback based error management over idiomatic error management.

I just found out that the way i suggested earlier for catching panics might not work because we haven't implemented std::panic::Unwindsafe trait for Array/&Array. We will look into this issue.

As i said earlier, in the code you shared earlier is failing to run dot function for two reasons:

  • dot expects two vectors as inputs
  • l0 is of type f32 where as sync0 is of type f64

which is the reason the af::dot call paniced due to lack of batch support (that is first argument validation before the check for type mismatch).

@ghost
Copy link
Author

ghost commented Feb 2, 2017

Thanks for the pointers on the mismatched types, that was an obvious error in the code. It would be very helpful if the error messages in Arrayfire helped to point out these issues when they occur. Unfortunately, I'm still getting exactly the same crash, even though everything is now f32, and the error messages still don't help. :/

For error management: right now, Arrayfire offers terse syntax, but at the expense of any safety. And, as a consumer of that code, my experience has been poor. I would happily sacrifice syntax, to take advantage of compile-time checks and more informative error messaging.

The Result<T, E> type has some really nice composibility methods that help to build complex logic without requiring .unwrap or .expect on everything. Perhaps if the Arrays supported some method-call syntactic sugar as an Impl it would clean things up a bit?

For example,

// This from above:
// (I'm not even sure this 
Array g = exp(add(div(a, b).unwrap(), mul(a, c).unwrap()), e);

// Perhaps alternately as this:
Array m = a.mul(c)  // Result<Array, Error>
Array d = a.div(b).then_add(m).then_exp(e)  // "then_XXX" methods use and return Result<Array, Error>

Internally, the then_XX methods would use Result<T,E> methods to elegantly handle unwrapping successful values, or passing-forward Error values. If errors occur, they get passed-through and caught at the end with an expect or unwrap call, or handled in any other usual way. If no error occurs, the array operations occur cleanly, and pretty expressively.

If panic-catching can get implemented, then perhaps a higher-abstraction on Arrayfire could provide the above by catching all runtime panics and returning Results instead, and adding type safety on top. What would be needed to implement panic-catches on all arrayfire methods?

@ghost ghost mentioned this issue Feb 2, 2017
@AtheMathmo
Copy link

I also wanted to chime in here and say that typed Arrays would be valuable to me too. Here is an example:

pub fn arr_grad(&mut self, wrt: Container<rugrads::Variable>) -> Array {
    let output_dims = wrt.inner().value(&self.0.context()).dims();
    let output_type = wrt.inner().value(&self.0.context()).get_type();
    let output_elms = output_dims.elements() as usize;

    match output_type {
        DType::F32 => {
            let ones = vec![1f32; output_elms];
            self.0.backprop(wrt, Array::new(&ones, output_dims))
        },
        DType::F64 => {
            let ones = vec![1f64; output_elms];
            self.0.backprop(wrt, Array::new(&ones, output_dims))
        },
        _ => panic!("Currently only float array types are supported")
    }
}

Instead of matching on the Array type I'd prefer to do something like:

pub fn arr_grad<T: num::One>(&mut self, wrt: Container<rugrads::Variable>) -> Array<T> {
    let output_dims = wrt.inner().value(&self.0.context()).dims();
    let output_type = wrt.inner().value(&self.0.context()).get_type();
    let output_elms = output_dims.elements() as usize;

    let ones = vec![T::one(); output_elms];
    self.0.backprop(wrt, Array::new(&ones, output_dims))
}

This way the compiler also helps me to remove the panic case above. This should also help to enforce type compatibility at compile time in the wrapper functions.


I think the current error handling is acceptable - I agree that returning results on every function call would be a mess. I'd argue that having typed Arrays actually helps this case as the Rust compiler helps to remove a whole class of ffi bugs that occur from feeding two differently typed arrays to functions.

In my mind this means the biggest remaining cause for runtime crashes would be dimensions mismatch like the one eluded to above for arrayfire::dot. I think it's fine to have these crashes for some functions, like addition. But we could consider fixing some of these issues by having the dimensionality of the Array encoded as a type parameter as well. You could take inspiration from bluss' ndarray crate.

This way you could use the dimension type information to enforce that arrayfire::dot takes one dimensional arrays.

To be clear I also echo the comments above - it is really awesome to have officially supported bindings like this. I'm having a lot of fun using them!

@ghost
Copy link
Author

ghost commented Feb 9, 2017 via email

@AtheMathmo
Copy link

@cathalgarvey - yes, it is open source. The project is still very much in its infancy but you can check it out. Right now you can automatically differentiate some elementwise operations - I'm going to try and get Rn->R functions (like loss functions) working soon.

Feel free to open an issue on the repo if you want to discuss it any further :)

@9prady9
Copy link
Member

9prady9 commented Feb 20, 2017

@cathalgarvey I have been investigating how to make this possible and i have hit a road block. You are welcome to provide inputs if there is some stable feature that i am not using that could make the implementation easier.

@jramapuram Any suggestions from you are also welcome.

After making Array typed. It looks like below.

pub struct Array<T> {
     handle: i64,
    _marker: std::marker::PhantomData<T>, 
}

trait IsArray {
    fn is_array() -> bool {
        false
    }
}

impl<T> IsArray for Array<T> {
    fn is_array() -> bool {
        true
    }
}

so far it works fine, i was even able to print the resulting Array<T>. However, the problem arises when the output array is not of the same type as input - yes, there are some functions where this happens.

pub fn sum<T: HasAfEnum>(input: &Array<T>, dim: i32) -> &IsArray {
    unsafe {
        let mut temp: i64 = 0;
        let err = af_sum(&mut temp as MutAfArray, input.get() as AfArray, dim as c_int);
        HANDLE_ERROR(AfError::from(err));
        let mut otype: c_int = 0;
        HANDLE_ERROR(af_get_type(&mut otype as *mut c_int, temp as AfArray));
        Array::<R>::from(temp)

/// How do we determine this R? or 
///How do we convert af_array handle from C FFI to `Array<R>` struct
/// otype can be retrieved and used to determine type ? some sort of match stmt ?
      }
}

I am stuck at how to determine the type of output Array even though i replaced the return type with a trait object.

@AtheMathmo
Copy link

AtheMathmo commented Feb 20, 2017

@9prady9

I am not sure why you would need to use a trait object here, why not just specify the return type in the function signature:

pub fn sum<T: HasAfEnum, R: HasAfEnum>(input: &Array<T>, dim: i32) -> Array<R> {
    unsafe {
        let mut temp: i64 = 0;
        let err = af_sum(&mut temp as MutAfArray, input.get() as AfArray, dim as c_int);
        HANDLE_ERROR(AfError::from(err));
        let mut otype: c_int = 0;
        HANDLE_ERROR(af_get_type(&mut otype as *mut c_int, temp as AfArray));
        Array::from(temp)
      }
}

Here's a dummy implementation independent of arrayfire.

The downside is that the user must specify the return type themselves - but if the return type truly is unknown and could be any of a set then this is probably the safest thing to do anyway. You could of course constrain the return type by using internal traits like: AFFloat for f64/f32 and similar things.

@9prady9
Copy link
Member

9prady9 commented Feb 20, 2017

@AtheMathmo Lets take the example of convolve function, you can see the input, output array types for it.

Input Array Type Output Array Type
complex float complex float
complex double complex double
double double
all other integral types float

So, the user can't write a general statement such as the below

let a = convolve(input);

He would have to write some thing like below

let a = match input.type() {
      DType::c32 => convolve::<Complex32, Complex32>(input);
      DType::c64 => convolve::<Complex64, Complex64>(input);
      DType::f64 => convolve::<f64, f64>(input);
      _ => convolve::<float, _>(input); // I am not sure if this `_` placeholder will work or we have to match all enum values explicitly
};

If such an expression is part of a larger mathematical equation, the code will look too verbose. This is the reason i refrained from taking the direction of using two generic types (one for input and the other for output).

Also, how does Array::from(temp) encode type parameter ? Given below is the From trait implementation i used earlier to generate a generic Array object. Does the type inference happen automatically if we just call Array::from(temp) and return an Array<R> ?

impl<R> From<i64> for Array<R> {
    fn from(t: i64) -> Array<R> {
        Array {handle: t, _marker: marker::PhantomData}
    }
}

@AtheMathmo
Copy link

I see the issue a little more clearly now. I am not sure if there is a really clean solution unless you build a complicate trait hierarchy in the background (which would probably be quite messy itself). One way I would consider solving this would be giving a separate function for the integral types.

trait AFFloat {}
impl AFFloat for {complex float}/{complex double}/{double}/{float} {}
trait AFInteger {}
impl AFInteger for ... {}

pub fn convolve<T: AFFloat>(input: Array<T>) -> Array<T> {}

pub fn iconvolve<I: AFInteger>(input: Array<I>) -> Array<f64> {}

Maybe there is a nicer way to solve this problem (like using specialization), but I feel like this is the only way without runtime type checking on stable.

Also, how does Array::from(temp) encode type parameter ?

The type inference does happen automatically, as the compiler knows the return type should have generic type R.

@9prady9
Copy link
Member

9prady9 commented Feb 20, 2017

Creating separate functions for integral types for all the functions in the library would lead to code bloat - which is one of the things we avoided earlier when we chose callback based error management over Rust's idiomatic error management. If we do this, then the argument of why not just use idiomatic error management will spring up again.

@jramapuram What's your take on having typed Array's and possible ways to implement ?

@AtheMathmo
Copy link

I suppose you could always do the trait object approach and do the runtime type matching within the convolve function:

pub fn convolve<T: HasAfEnum>(input: Array<T>) -> &IsArray {
    match input.type() {
        DType::c32 => convolve::<T, Complex32>(input);
        DType::c64 => convolve::<T, Complex64>(input);
        DType::f64 => convolve::<T, f64>(input);
        _ => convolve::<T, f64>(input);
    }
}

fn convolve_typed<T: HasAfEnum, R: HasAfEnum>(input: Array<T>) -> Array<R> {
    // ... Here we do the convolution with proper type matching
}

Or something like that...

Personally I would prefer to have the type information readily available in the function output but I understand the issues with this.

@9prady9 9prady9 added this to the timeless milestone Apr 27, 2017
@daboross
Copy link

daboross commented May 29, 2017

@9prady9 If Array was typed, I believe HasAFEnum could have a ConvolveOutput associated type so the user wouldn't specify it. Associated types are a type system 'output', so there'd only be one per input type and it should work correctly?

Like if you have trait HasAfEnum { type ConvolveOutput; }, you can specify the ConvolveOutput correctly for each input, then convolve is like:

pub fn convolve<T: HasAfEnum>(input: Array<T>) -> Array<T::ConvolveOutput> {
    // same code
}

If Array was typed, it would also make errors where types don't match much less common, correct?

ArrayFire seems like a really awesome project, I think it'd be great if it was easier to use correctly in Rust as well so it would be more discoverable and usable by Rust developers.

@polarathene
Copy link

For error management: right now, Arrayfire offers terse syntax, but at the expense of any safety. And, as a consumer of that code, my experience has been poor. I would happily sacrifice syntax, to take advantage of compile-time checks and more informative error messaging.

As much as I appreciate the terseness, I'd agree with the above wanting better typing with compile checks to avoid errors that cost me much more time to figure out with the limited examples and docs/community for the Rust wrapper. Several times I've been tripped up because a method takes my arrays and returns another of a different type than the variable I stored them to was, no error thrown, leaving me confused to what was wrong with my math/logic. Another one that has cost me a bit of time is panics/invalid values due to no borrow checking where the compiler could have let me know of the mistake.

I'll take verbosity in exchange for the safety Rust provides so I don't lose so much time to troubleshooting :)

@9prady9
Copy link
Member

9prady9 commented Jun 25, 2018

fyi - I have started working on typed arrays, hopefully I will have something usable by the end of this week or much earlier if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants