New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 906 - Broadcasting order of arguments
Summary: Broadcasting order of arguments
Status: DECISIONNEEDED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Documentation
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-12 20:30 UTC by nfoti01
Modified: 2014-11-14 21:37 UTC (History)
3 users (show)



Attachments

Description nfoti01 2014-11-12 20:30:22 UTC
I want to pointwise multiply the each row of a matrix by a vector which can easily be done with broadcasting as `A.rowwise() * a.transpose()` (where `A` and `a` are appropriately defined).  However, the mathematically equivalent expression `a.transpose * A.rowwise()` results in a compilation error saying that `operator*` is not defined for the types involved.

From Bug 157 it looks like the operator is just not implemented with the visitor on the right rather than a shortcoming of the expression generation.  However, this seems like a pretty high-priority issue to address since the expressions are mathematically the same and it seems ridiculous that people are forced to write them one way.  If these missing cases cannot be implemented then at the very least there should be clear documentation in the broadcasting section on this.
Comment 1 Gael Guennebaud 2014-11-13 22:14:15 UTC
Conceptually it is usually better to express this kind of operation as a non uniform scaling through the product with a diagonal matrix:

A * a.asDiagonal();

This is much closer to what you would write in a formula, and the generated code will be the same.

Regarding rowwise/colwise, the asymmetry is on purpose. A.rowwise() followed by something has to be read as for row of A do something. So I wonder whether adding what you suggest would not be counter-productive.
Comment 2 Christoph Hertzberg 2014-11-14 07:48:21 UTC
I'm not really sure what you intend to do. If `A` is a matrix and `a` a column vector, then the operation should not work according to the docu.
Maybe both are actually arrays? ('pointwise multiply' suggests they are).

Generally, I would read `A.rowwise() OPERATOR B` as `A rowwiseOperator B`, which I would interpret as (assuming all sizes are appropriate):
  for(int r=0; r<A.rows(); ++r) result.row(r) = A.row(r) OPERATOR a;

Allowing your suggestion will likely lead to strange behavior if you write strange expressions such as
  A OPERATOR B.rowwise() OPERATOR C; 
which operation is rowwise? Shall there be a runtime error if sizes don't fit?
If you really like to swap the order of `A` and `a` in your example, you can use some kind of .replicate() syntax instead.

However, I am totally open for any clarifications in the documentation. Perhaps adding something to http://eigen.tuxfamily.org/dox-devel/group__TutorialReductionsVisitorsBroadcasting.html or linking to that page more prominently.
Comment 3 nfoti01 2014-11-14 08:12:35 UTC
The asDiagonal suggestion would be fine, but it doesn't work for Arrays.  If it's a design decision that the visitor needs to come first then this needs to be prominently documented.

Regarding the example of strange behavior from strange expressions, I'm not sure why the behavior would be strange.  Operator precedence will determine an evaluation of the expression, it may not be what the user wanted, but that's what parentheses are for.  Maybe replicate should be linked from the broadcasting section so that people are aware it can be useful for broadcasting.

It's fine if Eigen deviates from math, but these deviations need to be clearly documented.
Comment 4 Gael Guennebaud 2014-11-14 12:14:50 UTC
Christoph expressed in a better way what I wanted to say.

OK to improve the doc.
Comment 5 Christoph Hertzberg 2014-11-14 21:37:00 UTC
(In reply to nfoti01 from comment #3)
> The asDiagonal suggestion would be fine, but it doesn't work for Arrays.  If
> it's a design decision that the visitor needs to come first then this needs
> to be prominently documented.

Yes, asDiagonal() only works for matrices, but you asked about matrix and vector in your first post (thus our confusion).

> Regarding the example of strange behavior from strange expressions, I'm not
> sure why the behavior would be strange.  Operator precedence will determine
> an evaluation of the expression, it may not be what the user wanted, but
> that's what parentheses are for. 

Sure, in C++ multiplication is left-associative, so there is no ambiguity. But mathematically multiplication is (usually) associative, so it would be confusing if (A*B.rowwise())*C results in a run-time error, whereas A*(B.rowwise()*C) works.

> Maybe replicate should be linked from the
> broadcasting section so that people are aware it can be useful for
> broadcasting.

Adding information about replicate is definitely a good idea.

> It's fine if Eigen deviates from math, but these deviations need to be
> clearly documented.

The rowwise()/colwise() broadcasting alone is an extension of Eigen -- in "math", I would usually write it with some indexed expression, i.e. R_{ij}=A_{ij}*a_j for R=A.rowwise()*a.

Note You need to log in before you can comment on or make changes to this bug.