Skip to content
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

set_col() fails #143

Closed
polarathene opened this issue Jun 9, 2017 · 8 comments
Closed

set_col() fails #143

polarathene opened this issue Jun 9, 2017 · 8 comments

Comments

@polarathene
Copy link

polarathene commented Jun 9, 2017

set_col() docs could do with an example.

It is not clear what the expected array for new_col should look like. I have tried dims [3, 1, 1, 1] and [1, 3, 1 ,1], both times an error was thrown about incorrect size.

let range_a: Vec<u8> = (1..4).collect();
let range_b: Vec<u8> = (4..7).collect();

let dims = Dim4::new(&[1, 3, 1, 1]);
let mut range_a_af = af::Array::new(&range_a, dims);
let     range_b_af = af::Array::new(&range_b, dims);

af_print!("range_a_af:", range_a_af);
// [1 3 1 1]
//          1          2          3 
af_print!("range_b_af:", range_b_af);
// [1 3 1 1]
//          4          5          6

// I'd like all values in a column to be changed to those the values in the given array or the given arrays matching column, I assume for a scalar value I'd use a constant array(sized to elements in the column or matching dims depending how this method works
range_a_af = set_col(&range_a_af, &range_b_af, 0); // should replace 1 with 4?
af_print!("range_a_af == [4, 2, 3]?: ", range_a_af);

This throws the following error:

In function void assign(opencl::Array<T>&, const unsigned int&, const af_seq*, const opencl::Array<outType>&) [with Tout = unsigned char; Tin = unsigned char; af_seq = af_seq]
In file src/api/c/assign.cpp:59
Size mismatch between input and output

thread 'main' panicked at 'Error message: Size is incorrect', /home/pola/.cargo/registry/src/github.com-1ecc6299db9ec823/arrayfire-3.4.3/src/error.rs:66

This might be due to the indexing bug that @9prady9 fixed here. Either that or I've misunderstood how to use this method.

@9prady9
Copy link
Member

9prady9 commented Jun 9, 2017

@polarathene I believe you misunderstood what column means here. Given that your input is [1 3 1 1], there are three columns in the Array and each column has single element. Therefore, the new_col parameter of set_col should be an Array of size 1. Hence, you are seeing an error - your new_col Array is of again [1 3 1 1] size where as the function is expecting [1 1 1 1].

I agree that an example in the functions documentation would definitely and we have an open issue for the same.

@9prady9 9prady9 closed this as completed Jun 9, 2017
@polarathene
Copy link
Author

If there were 5 rows of 3 columns([5, 3, 1, 1]), then the new_col dims would be [5,1,1,1]?

@9prady9
Copy link
Member

9prady9 commented Jun 9, 2017

Yes, that is correct.

Array with dimensions [ M N O P ]
has

  • N columns
  • M rows
  • O slices of size MxN

M is number of element in a column
N is number of elements in a row
O is number of elements in a slice of MxN elements

Thus the naming convention of set_[row | col | slice] and get_[row | col | slice] functions.

@polarathene
Copy link
Author

polarathene commented Jun 9, 2017

I was familiar with row and col, I noticed slice but it didn't click to me that it was related to these two like that for some reason :) I guess the new docs will mention the 3 together like you just did.

I put together an example for using set_col() and col()(Rust wrapper doesn't call it get_col()).

// Showcases usage of `set_col()` and `col()`
fn col_example() {
    let range_a: Vec<u8> = (1..16).collect();
    let range_b: Vec<u8> = (20..26).collect();
    let range_c: Vec<u8> = (30..41).collect();

    // The arrays columns must have the same size(number of rows)
    let dims_a = Dim4::new(&[5, 3, 1, 1]);
    let dims_b = Dim4::new(&[5, 1, 1, 1]);
    let dims_c = Dim4::new(&[5, 2, 1, 1]);

    let mut range_a_af = af::Array::new(&range_a, dims_a);
    let     range_b_af = af::Array::new(&range_b, dims_b);
    let     range_c_af = af::Array::new(&range_c, dims_c);



    af_print!("range_a_af:", range_a_af);
    // [5 3 1 1]
    //      1          6         11
    //      2          7         12
    //      3          8         13
    //      4          9         14
    //      5         10         15
    af_print!("range_b_af:", range_b_af);
    // [5 1 1 1]
    //     20 
    //     21 
    //     22 
    //     23 
    //     24 
    af_print!("range_c_af:", range_c_af);
    // [5 2 1 1]
    //     30         35 
    //     31         36 
    //     32         37 
    //     33         38 
    //     34         39 



    // `range_b_af` only contains 1 column of 5 rows. To replace the first
    // column in `range_a_af`(0):
    range_a_af = set_col(&range_a_af, &range_b_af, 0);

    af_print!("1st column replaced: ", range_a_af);
    // [5 3 1 1]
    //     20          6         11 
    //     21          7         12 
    //     22          8         13 
    //     23          9         14 
    //     24         10         15 



    // Take the 2nd column from `range_c_af`(1) and replace the 3rd column
    // in `range_a_af`(2):
    range_a_af = set_col(&range_a_af, &col(&range_c_af, 1), 2);

    af_print!("3rd column replaced: ", range_a_af);
    // [5 3 1 1]
    //     20          6         35 
    //     21          7         36 
    //     22          8         37 
    //     23          9         38 
    //     24         10         39 
}

@polarathene
Copy link
Author

Rust docs for set_col() I think just need brief code snippet and perhaps mention:

The arrays columns must have the same size(number of rows)

@9prady9
Copy link
Member

9prady9 commented Jun 9, 2017

@polarathene My bad, I guessed it is get_*. Can you please send in a PR with the code example added to the function documentation ?

@polarathene
Copy link
Author

code example added to the function ?

I've not contributed rust docs before. Is the docs contained within the source? Or did you want the larger snippet in the examples? Happy to send a PR.

@9prady9
Copy link
Member

9prady9 commented Jun 9, 2017

Documentation would be part of source code. You can look at the example written for shift to get an idea of how to write an example.

Also, once you write a test - the command cargo test will run the code stub you add to the documentation - this way you can check that the code snippet you add compiles fine. You can always ping me on gitter if you get stuck anywhere.

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

No branches or pull requests

2 participants