Repeat operation #2090
Repeat operation #2090
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2090 +/- ##
==========================================
+ Coverage 86.18% 86.23% +0.04%
==========================================
Files 687 688 +1
Lines 88187 88465 +278
==========================================
+ Hits 76008 76285 +277
- Misses 12179 12180 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looks good but I have a few requests:
- The book doc needs to be updated.
- The doc strings should be updated with what arguments are passed.
- Double check if your unit tests do not produce false positives. I linked a bug. So if you data type or shape do not match in addition to values, you'll have wrong assertion.
| Self::new(K::repeat_dim(self.primitive, dim, times)) | ||
| } | ||
|
|
||
| /// Repeat the tensor along the given dimensions. |
There was a problem hiding this comment.
We need to describe arguments in the string docs.
| let output = tensor.repeat_dim(0, 4); | ||
| let expected = TensorData::from([[0, 1, 2], [0, 1, 2], [0, 1, 2], [0, 1, 2]]); | ||
|
|
||
| output.into_data().assert_eq(&expected, false); |
There was a problem hiding this comment.
Beware of the false positives bug: #1982
| [[13.0, 14.0, 13.0, 14.0], [15.0, 16.0, 15.0, 16.0]], | ||
| ]); | ||
|
|
||
| output.into_data().assert_eq(&expected, false); |
There was a problem hiding this comment.
Beware of the false positives bug: #1982
| [[13, 14, 13, 14, 13, 14], [15, 16, 15, 16, 15, 16]], | ||
| ]); | ||
|
|
||
| output.into_data().assert_eq(&expected, false); |
There was a problem hiding this comment.
Beware of the false positives bug: #1982
| | `tensor.repeat_dim(2, 4)` | `tensor.repeat([1, 1, 4])` | | ||
| | `tensor.repeat(&[(0,2),(2,4)])` | `tensor.repeat([2, 1, 4])` | |
There was a problem hiding this comment.
To be consistent we should describe params being passed just like other examples in this doc. Also the pytorch equivalent examples should match. For example, tensor.repeat(&[(0,2),(2,4)]) is not same as tensor.repeat([2, 1, 4])
| [[1.0f32, 2.0f32], [3.0f32, 4.0f32]], | ||
| [[5.0f32, 6.0f32], [7.0f32, 8.0f32]], | ||
| [[9.0f32, 10.0f32], [11.0f32, 12.0f32]], | ||
| [[13.0f32, 14.0f32], [15.0f32, 16.0f32]], |
There was a problem hiding this comment.
No need to change this but it's sufficient to specify type once (at the beginning) and rust can infer the types.
Pull Request Template
Checklist
run-checks allscript has been executed.Related Issues/PRs
Closes #1715
Changes
Implementing multi-dimensional repeat operation for burn tensor. Renamed repeat to repeat_dim to better illustrate that repeat_dim only works on a single dimension.
Testing
Added unit tests to verify the correctness of the operation