Add Repeat and Replicate ANN Layer Types #3565

akropp · 2023-11-27T20:22:41Z

Adding two ANN layer types:

RepeatType -- Repeat input tensors along one dimension n times, similar to the arma::repelem function or tensorflow.repeat
ReplicateType -- Replicate the input in a block-like fasion across each dimension a specified number of times, similar to arma::repmat or tensorflow.tile

rcurtin

Thanks, these are important and nice layers to add. Sorry it took me so long to get to this review. I only did a quick pass so far---most of my specific comments are style-related, or comment-related.

The two classes are very similar. I wonder if it makes more sense to the user if we just create one layer Repeat, and whether it interleaves (like repelem()) or block copies (like repmat()) is a boolean parameter. What do you think?

src/mlpack/methods/ann/layer/repeat.hpp

src/mlpack/methods/ann/layer/replicate.hpp

src/mlpack/methods/ann/layer/layer_types.hpp

akropp · 2023-12-12T12:27:03Z

Thanks, these are important and nice layers to add. Sorry it took me so long to get to this review. I only did a quick pass so far---most of my specific comments are style-related, or comment-related.

The two classes are very similar. I wonder if it makes more sense to the user if we just create one layer Repeat, and whether it interleaves (like repelem()) or block copies (like repmat()) is a boolean parameter. What do you think?

I definitely had that thought. It seemed that the two concepts are kept separate with some other frameworks, so I was just sort of mirroring that. I only actually had use for the Replicate one, but figured I'd add both for completeness. Could go either way.

Co-authored-by: Ryan Curtin <[email protected]>

akropp · 2023-12-12T16:42:47Z

Hm. Perceptron test is failing on MacOS. Is this a known issue? Obviously not related to this commit. And it succeeds on my local machine (which is MacOS, and same Armadillo version) for what it's worth.

akropp · 2023-12-13T18:47:17Z

Is the perceptron test failing on other branches?

2023-12-13T17:58:25.0665010Z -------------------------------------------------------------------------------
2023-12-13T17:58:25.0665220Z IncrementalTrainingTest
2023-12-13T17:58:25.0665540Z -------------------------------------------------------------------------------
2023-12-13T17:58:25.0665820Z /Users/runner/work/1/s/src/mlpack/tests/perceptron_test.cpp:289
2023-12-13T17:58:25.0666070Z ...............................................................................
2023-12-13T17:58:25.0666180Z 
2023-12-13T17:58:25.0666360Z /Users/runner/work/1/s/src/mlpack/tests/perceptron_test.cpp:319: FAILED:
2023-12-13T17:58:25.0666670Z   REQUIRE( !approx_equal(p1.Weights(), p2.Weights(), "absdiff", 1e-5) )
2023-12-13T17:58:25.0666850Z with expansion:
2023-12-13T17:58:25.0667000Z   false

This is on the MacOS builds. Doesn't fail on my mac with same armadillo version.

Co-authored-by: Ryan Curtin <[email protected]>

…to replicate_layer

Co-authored-by: Ryan Curtin <[email protected]>

akropp · 2024-01-06T01:32:10Z

When the decision_tree_regressor test fails, this is the output:

/home/vsts/work/1/s/src/mlpack/tests/decision_tree_regressor_test.cpp:955: FAILED:
REQUIRE( rmse < 6.1 )
with expansion:
6.1651240525 < 6.1

Any idea of 6.1 was chosen with some mathematical reasoning (as opposed to empirically chosen)? If not, any objection to changing it to 6.2?

rcurtin · 2024-01-06T16:09:04Z

Sorry, forgot to mention it here, but I opened #3594 to fix the decision tree regressor issues. If I had to guess, the tolerance was originally 6.0 (set somewhat arbitrarily by observation), then bumped to 6.1, and I think in #3594 I bumped to 6.2 and/or allowed multiple trials for it. Anyway, there were no failures in that test suite in 1000 trials, so if you merge master into this branch, at least that test should be fixed. :)

…to replicate_layer

src/mlpack/methods/ann/layer/repeat_impl.hpp

shrit

Looks good to me 👍

src/mlpack/methods/ann/layer/repeat.hpp

This reverts commit 0253a86.

Signed-off-by: Omar Shrit <[email protected]>

shrit · 2024-01-18T18:55:40Z

src/mlpack/core/util/arma_traits.hpp

+template<typename MatType>
+struct GetUIntColType
+{
+  typedef typename MatType::elem_type eT;
+  typedef typename std::conditional<
+    std::is_same<eT, arma::uword>::value, eT, arma::uword>::type elem;
+  typedef arma::Col<elem> type;
+};
+


@rcurtin I tried this, I do not know if you might like it or not

Regspace should be used on vectors, however, in the following we were assigning it to a matrix instead and then calling reshape. Now when using templates this is no longer possible, therefore we need to explicitly call conv_to to convert it from col to matrix. Signed-off-by: Omar Shrit <[email protected]>

rcurtin

@shrit just some minor comments. If you can handle those then from my side I think everything is ready to merge. @akropp sorry this has been held up so long, thanks again for the implementation! I know we made a couple of changes after you were done so let us know if we accidentally broke any of your ideas or anything.

src/mlpack/core/util/arma_traits.hpp

src/mlpack/methods/ann/layer/repeat_impl.hpp

Signed-off-by: Omar Shrit <[email protected]>

akropp and others added 6 commits November 22, 2023 17:35

Add ANN Replicate Layer

f2a39c6

Add Replicate Layer and Test Case

1653a1c

Make replicate work across multiple dimensions at once. Add test cases.

ab989a6

Update layer_types.hpp

82e3233

Update layer_test.cpp

ba4fd2b

Wrapping

b024ae6

mlpack-bot bot added s: needs review s: unanswered s: unlabeled labels Nov 27, 2023

Make repeat interface the same as replicate. Fix test cases.

33aa0ff

rcurtin reviewed Dec 11, 2023

View reviewed changes

akropp and others added 7 commits December 12, 2023 07:27

Update src/mlpack/methods/ann/layer/replicate.hpp

887673f

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/methods/ann/layer/repeat.hpp

d0c34dc

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/methods/ann/layer/repeat.hpp

385e939

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/methods/ann/layer/repeat.hpp

7bec04a

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/methods/ann/layer/repeat.hpp

c153829

Co-authored-by: Ryan Curtin <[email protected]>

Style Changes, Documentation, Add to serialization

81c2af3

Doc

39e158b

docs

991b329

akropp and others added 8 commits December 13, 2023 14:52

Add ANN Replicate Layer

ff98744

Add Replicate Layer and Test Case

8b1035e

Make replicate work across multiple dimensions at once. Add test cases.

48e9a38

Update layer_types.hpp

4709f39

Update layer_test.cpp

69e45ec

Wrapping

e3ea704

Make repeat interface the same as replicate. Fix test cases.

7c2e2e1

Update src/mlpack/methods/ann/layer/replicate.hpp

e37cf40

Co-authored-by: Ryan Curtin <[email protected]>

akropp and others added 9 commits January 5, 2024 17:19

Update test_catch_tools.hpp

1807e8e

Merge branch 'replicate_layer' of https://github.com/akropp/mlpack in…

4beb301

…to replicate_layer

Proxy mat params so we don't have to copy unless necessary.

d275d2e

Update src/mlpack/tests/test_catch_tools.hpp

5878cdf

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/tests/test_catch_tools.hpp

c5dbb14

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/tests/test_catch_tools.hpp

560eef5

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/tests/test_catch_tools.hpp

d262daa

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/tests/test_catch_tools.hpp

2931e2e

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/tests/test_catch_tools.hpp

69a15da

Co-authored-by: Ryan Curtin <[email protected]>

akropp added 2 commits January 8, 2024 07:37

remove arma:: prefixes on functions

259695b

Merge branch 'replicate_layer' of https://github.com/akropp/mlpack in…

41e93cf

…to replicate_layer

akropp requested a review from shrit January 8, 2024 14:15

shrit reviewed Jan 9, 2024

View reviewed changes

src/mlpack/methods/ann/layer/repeat_impl.hpp Outdated Show resolved Hide resolved

src/mlpack/methods/ann/layer/repeat_impl.hpp Outdated Show resolved Hide resolved

Remove last references to arma::

0253a86

shrit approved these changes Jan 10, 2024

View reviewed changes

mlpack-bot bot removed the s: needs review label Jan 10, 2024

rcurtin reviewed Jan 11, 2024

View reviewed changes

src/mlpack/methods/ann/layer/repeat.hpp Outdated Show resolved Hide resolved

shrit added 4 commits January 18, 2024 18:04

Merge branch 'master' into replicate_layer

87d753d

Revert "Remove last references to arma::"

eedefa1

This reverts commit 0253a86.

Fix the type, use generic type instead of arma

d7d67bd

Signed-off-by: Omar Shrit <[email protected]>

Make it as generic as possible

4aef673

Signed-off-by: Omar Shrit <[email protected]>

shrit reviewed Jan 18, 2024

View reviewed changes

rcurtin approved these changes Jan 26, 2024

View reviewed changes

src/mlpack/core/util/arma_traits.hpp Show resolved Hide resolved

src/mlpack/core/util/arma_traits.hpp Show resolved Hide resolved

src/mlpack/core/util/arma_traits.hpp Show resolved Hide resolved

src/mlpack/methods/ann/layer/repeat_impl.hpp Outdated Show resolved Hide resolved

Follow the suggestions

e64481c

Signed-off-by: Omar Shrit <[email protected]>

shrit merged commit 1edb179 into mlpack:master Jan 28, 2024

This was referenced May 26, 2024

Release version 4.4.0 #3716

Closed

Release version 4.4.0 #3717

Merged

Uh oh!

Add Repeat and Replicate ANN Layer Types #3565

Add Repeat and Replicate ANN Layer Types #3565

Uh oh!

Conversation

akropp commented Nov 27, 2023

Uh oh!

rcurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akropp commented Dec 12, 2023

Uh oh!

akropp commented Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akropp commented Dec 13, 2023

Uh oh!

akropp commented Jan 6, 2024

Uh oh!

rcurtin commented Jan 6, 2024

Uh oh!

Uh oh!

Uh oh!

shrit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shrit Jan 18, 2024

Choose a reason for hiding this comment

Uh oh!

rcurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akropp commented Dec 12, 2023 •

edited

Loading