[Add] broadcasting for Arithmetic Operators
This merge request is dedicated to enabling broadcasting on binary operators (Add, Mul, Div, Sub, Pow). The expected behaviour is the same as numpy
Here are examples of supported broadcasting
| Tensor_A shape | 3 | 2 | 1 | 3 |
|---|---|---|---|---|
| Tensor_B shape | 1 | 2 | 4 | 3 |
| Output shape | 3 | 2 | 4 | 3 |
| Tensor_A shape | 3 | 2 | 1 | 3 |
|---|---|---|---|---|
| Tensor_C shape | 3 | |||
| Output shape | 3 | 2 | 1 | 3 |
Merge request reports
Activity
assigned to @hrouis
requested review from @pineapple
changed milestone to %v0.1.0
changed milestone to %aidge_core - v0.2.0
added 79 commits
-
54915f36...a5fb8430 - 78 commits from branch
eclipse/aidge:main - 173a958a - Merge branch aidge_core:main into broadcasting
-
54915f36...a5fb8430 - 78 commits from branch
added 1 commit
- 9097e81f - make ArithmeticOperator inherit from OperatorTensor
added 3 commits
-
9097e81f...b9d7bf31 - 2 commits from branch
eclipse/aidge:main - 19f75fbe - Merge branch aidge_core:main into broadcasting
-
9097e81f...b9d7bf31 - 2 commits from branch
added Feature 🚀 LanguageC++ labels
mentioned in merge request !76 (merged)
added 14 commits
Toggle commit list
src/operator/ArithmeticOperator.cpp 0 → 10064423 24 void Aidge::ArithmeticOperator::computeOutputDims() { 25 // check inputs have been associated 26 if (!getInput(0) || !getInput(1)) { 27 AIDGE_THROW_OR_ABORT(std::runtime_error, "At least one input was not connected"); 28 } 29 30 // if (getInput(0)->empty() || getInput(1)->empty()) { 31 // AIDGE_THROW_OR_ABORT(std::runtime_error, "At least one input is empty"); 32 // } 33 34 std::vector<std::vector<std::size_t>> inputsDims; 35 for (std::size_t i = 0; i < nbInputs(); i++) 36 { 37 inputsDims.push_back(getInput(i)->dims()); 38 } - Comment on lines +34 to +38
Ypu already know there are two inputs only for this kind of operators.
Agree with @pineapple, but also, just to be safe shouldn't we add an assert on the nb of inputs ?
changed this line in version 12 of the diff
- Resolved by Houssem ROUIS
I am not sure it is a good idea to add the
ArithmeticOperatorsub class.This class only update the behavior of
computeOutputDims, I would be more in favor of copy pasting this function in the different operators.I think we should avoid to add to much layer of inheritance in the code as it can hurt code readability and in this case the code duplication is not substancial.
- Last reply by Houssem ROUIS
-
This MR is related to #64 (closed)
added 37 commits
-
c67e943c...95077929 - 34 commits from branch
eclipse/aidge:dev - 5cf910bf - Merge branch 'dev' into broadcasting
- 1b16db67 - [Add] Scalar constructor for Tensor and int8 support
- d6e0e879 - [Add][WIP] Test for 'Div::computeOutputDims()' memeber function
Toggle commit list-
c67e943c...95077929 - 34 commits from branch
added 83 commits
-
d6e0e879...b3f36f6b - 82 commits from branch
eclipse/aidge:dev - 2231e352 - Merge branch 'dev' into broadcasting
-
d6e0e879...b3f36f6b - 82 commits from branch
added PriorityMedium StatusReview Ready TopicOperator labels
added Missing label
assigned to @pineapple
mentioned in commit 0b990efd
mentioned in issue #64 (closed)
mentioned in merge request !105 (merged)