Dimension calculation in CWiseSum does not quite make sense
- Dominant language
- C++
- Stars
- 3.4k
- Forks
- 701
- PR merge metrics
- No merged PRs in 30d
Description
The logic behind `dim_forward` function in `CWiseSum` (https://github.com/clab/dynet/blob/master/dynet/nodes-arith-cwise.cc#L20-L34) is a bit weird.
Consider two expressions A and B, whose dimensions are (1, 3) and (3,) respectively. When evaluating A + B, current logic would give the resulting dimension of (3, 3). This is somewhat counter-intuitive, as what I would expect is either to squeeze A and do elem-wise add, or to raise an exception.
I think the reasonable way of doing things here is to check whether: every dim of the "smaller" expression is equal to either 1 or the dim of the other expression.
PS. A minimum example:
``` python
import dynet as dy
x = dy.reshape(dy.inputTensor([1, 2, 3]), (1, 3))
y = dy.inputTensor([1, 2, 3])
z = x + y
print z.dim() # ((3, 3), 1)
print z.value()
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in dynet/nodes-arith-cwise.cc at CWiseSum::dim_forward and reproduce the supplied Python example to observe the current (3, 3) result. Compare the dimension rule with the proposed validation, then confirm that the example either follows the intended element-wise behavior or raises an appropriate exception.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100