arrayfire / arrayfire-rust

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

Controversial seq! macro usage examples #301

Closed videobitva closed 2 years ago

videobitva commented 2 years ago

Hello!

The seq! macro usage example, which could be found here, is pretty much controversial and made me frustrating for several hours while trying to figure out what is wrong with me and my coding skills.

Take a look at this section:

let b = constant(1.0 as f32, dim4!(3, 3));
let seqs = [seq!(1:3:1), seq!()];
assign_seq(&mut a, &seqs, &b);

Here we can clearly see the way to use this macro:

seq!(start:stop:step);

However, this does not work, because the way it supposed is to be (according to reality and this doc page) is to be like this:

seq!(start,stop,step);

As this issue name has controversial in it, time to take a look at this bit of docs:

let seqs = &[Seq::new(1u32, 3, 1), Seq::default()];
let _sub = index(&a, seqs);
//af_print!("a(seq(1,3,1), span)", sub);
// [3 5 1 1]
//     0.6720     0.3932     0.0621     0.9159     0.8434
//     0.5339     0.2706     0.7089     0.0231     0.1328
//     0.1386     0.9455     0.9434     0.2330     0.2657

Here the correct way of code usage was given, but the problem is that it was commented out, thus while looking through you can easily miss it. If we continue reading, we can easily see (as we already discovered it), that every single example of the correct way of usage of this macro was commented out, while the wrong way was kept.

Of course, all of this would not be a problem if I firstly looked through the docs here, but, the thing is, in the first section it clearly states that you should use the tutorials, which have misleading info in it. Please correct the tutorial so there will be less people in pain.

videobitva commented 2 years ago

After some time I once again had a thought that it could be me doing something wrong with given me instruments, so, just in case, here is my code I was working with:

let img: Array<u8> = load_image("some/path/gray.png", false);
let seq1 = seq!(1:height - 1:1);
let seq2 = seq!(1:width - 1:1);
let cropped = view!(img[seq1, seq2]);

If I try to compile it, I will have such output:

error: expected type, found `1`
   --> src/on_fire.rs:16:41
    |
16  |     let seq1 = seq!(1:height - 1:1);
    |                                        -^ expected type
    |                                        |
    |                                        tried to parse a type due to this
    | 
   ::: /usr/user/.cargo/registry/src/github.com-1ecc6299db9ec823/arrayfire-3.8.0/src/core/macros.rs:156:6
    |
156 |     ($start:expr , $end:expr , $step:expr) => {
    |      ----------- while parsing argument for this `expr` macro fragment

If I replace, for example, seq!(1:height - 1:1) with seq!(1, height - 1, 1), everything works just fine.

What is interesting is that tutorial examples compile, but trying to use the same techniques in my code fails.

9prady9 commented 2 years ago

@videobitva Sorry about the delay in my response.

There are variations in syntax due to the limitation on how : could be interpreted in a macro when expr type is used. So, I couldn't use : colon as delimiter for all cases. : delimiter can only be used with literals. When expressions are passed in as the sequence macro arguments, I use , as delimiter.

I think to avoid confusion, perhaps I should switch to , as delimiter in all cases. Another thing would be to add some examples to tutorial with expression based arguments rather than constant literals.

Hence, this is not a bug but rather just difference in syntax of seq macros when arguments are expressions vs constant literals.

9prady9 commented 2 years ago

Hope the clarification cleared the confusion. I have also opened a new issue to fix the consistency problem in seq macro. Please follow it's updates over there.

https://github.com/arrayfire/arrayfire-rust/issues/306