Why can't expressions be used in comparisons?

[grothesque]

Mdarray’s test function test_ops contains the lines

    a = expr::fill(1) - a;
    a = &b - a;

The variables a and b are of typeDTensor<i32, 2>.

I noticed that changing the second line to

    a = &b - expr::fill(1);

does not compile while

    a = b.clone() - expr::fill(1);

does (but consumes the lhs operand).

I wonder if this is this due to design, limitation, or perhaps simply omission?

[grothesque]

OK, I understand now that in

    a = &b - expr::fill(1);

the RHS evaluates to an expression which cannot be moved into a DTensor. Conversly in

    a = b.clone() - expr::fill(1);

the RHS evaluates to a DTensor. This is possible because the expression is allowed to consume (and thus reuse) b.clone().

This direct control of allocations is actually very nice!

So the solution is to write

    a.assign(&b - expr::fill(1));

---

However, my actual problem was that I was trying to compare a tensor with an expression, like this

    assert!(c == (&a + &b));

This does not work because as the documentation says “for comparison operators, the parameters must always be arrays that are passed by reference.” It can be solved by using .eval() on the expression, but this leads to an allocation.

So the question which remains is: why is it that expressions cannot be used with comparison operators?

[fre-hu]

One issue is that comparison operators take references to the arguments. Since an expression works like an iterator, it can only be evaulated once and is then consumed.

A first step could be to add eq() and ne() methods in the Expression trait like for iterators. Then the example can be written as

assert!(c.expr().eq(&a + &b));

or simpler

assert!((&a + &b).eq(&c));

It is possible to implement comparision operators for expressions that are for example Copy or Clone, but I'm not sure about that.

[grothesque]

Thanks for the explanation!

What is actually the reason that expressions can only be evaluated once? The usages of expressions that I am aware of (e.g. fill, from_fn, map) do not seem to require this property.

I understand the inner working of expressions only partially. Expressions derive from IntoIterator (which would speak for the non-repeatable aspect), but then implementers also have to provide a get_unchecked function.

By the way, how would one realize a mdarray expression that takes a shape and an iterator over values and “fills” the array with these values in row-major order?

[fre-hu]

It is to be able to have expressions that move out of an array, like into_expr() or drain(). But it is quite common as your examples that the expression can implement Copy and then it is not a limitation.

To fill an array, one way is to convert the array to a slice (with array[..]) and fill with the iterator directly. It only works if the array is contiguouos though. It would be possible to have something like fn expr::from_iter(shape, iter) that creates an expression so that it can be used in more cases.

[fre-hu]

I have added comparison methods now, see the latest commit.

[grothesque]

Thanks!

I wanted to try it out in mdarray-linalg-prototype. Here is a diff on top of the current main:

diff --git a/Cargo.toml b/Cargo.toml
index 62be2b8..84d5025 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -5,7 +5,7 @@ edition = "2024"
 
 [dependencies]
 cblas-sys = "0.2.0"
-mdarray = { git = "https://github.com/fre-hu/mdarray.git", rev = "2a67f2ec7ed326fad9dc95a94f772b7e2140c8eb" }
+mdarray = { git = "https://github.com/fre-hu/mdarray.git", rev = "3eca867a9c038e66fd8a04799857b8aee27e4900" }
 num-complex = "0.4.6"
 paste = "1.0.15"
 
diff --git a/tests/context.rs b/tests/context.rs
index eb6260e..23f4c73 100644
--- a/tests/context.rs
+++ b/tests/context.rs
@@ -19,7 +19,8 @@ fn test_backend(bd: &impl MatMul<f64>) {
 
     c.assign(c_expr());
     bd.matmul(&a, &b).add_to(&mut c);
-    assert!(c == ab.clone() + c_expr());
+    let temp = &ab + c_expr();
+    assert!(c.eq(&temp.eval()));
 
     c.assign(c_expr());
     bd.matmul(&a, &b).add_to_scaled(&mut c, 2.0);

The above works, but it does not avoid the clone. The following variants (etc.) all do not seem to work

assert!(c.eq(&temp));
assert!(c.eq(temp));
assert!(c.expr().eq(temp));
assert!(c.expr().eq(&temp));

I’d be grateful for any suggestions.

[fre-hu]

What error do you get for the second variant, is it that c gives references but not temp?

[grothesque]

error[E0308]: mismatched types
   --> mdarray-linalg-prototype/tests/context.rs:23:18
    |
23  |     assert!(c.eq(temp));
    |               -- ^^^^ expected `&_`, found `Map<Zip<FromFn<..., ...>, ...>, ...>`
    |               |
    |               arguments to this method are incorrect
    |
   ::: mdarray-linalg-prototype/tests/common/mod.rs:8:35
    |
8   | ) -> expr::FromFn<(usize, usize), impl FnMut(&[usize]) -> f64> {
    |                                   --------------------------- the found opaque type
    |
    = note: expected reference `&_`
                  found struct `Map<Zip<FromFn<(usize, usize), ...>, ...>, ...>`
note: method defined here
   --> /home/cwg/cb/autostow/rust/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cmp.rs:254:8
    |
254 |     fn eq(&self, other: &Rhs) -> bool;
    |        ^^
    = note: the full name for the type has been written to '/home/cwg/wo/24/mdarray/target/debug/deps/context-b589364f1e918a4c.long-type-14746268140945401265.txt'
    = note: consider using `--verbose` to print the full type name to the console
help: consider borrowing here
    |
23  |     assert!(c.eq(&temp));
    |                  +

For more information about this error, try `rustc --explain E0308`.
error: could not compile `mdarray-linalg-prototype` (test "context") due to 1 previous error

[fre-hu]

Ah it is the wrong eq method, it should be the third or fourth variant. Maybe it can be fixed by adding IntoExpression for &expr that gives references instead of values.