clab / clab/dynet

Dimension calculation in CWiseSum does not quite make sense

Open
#1,078 1 comment 0 reactions 0 assignees View on GitHub
moderate bug
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.