DemonSGD & DemonAdam #211

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

rcurtin · 2020-09-28T22:25:56Z

Let's keep this open---I have been meaning to review it but haven't quite gotten to it yet. 👍

mlpack-bot · 2020-10-28T23:12:42Z

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

rcurtin

Nice, looks good! Only one real comment about the default value for T. It's surprising both that the technique seems to yield such nice results, and also that the authors did such comprehensive comparisons... most of these papers they seem to only compare with plain SGD on logistic regression on MNIST...

rcurtin · 2020-12-01T19:22:45Z

doc/optimizers.md

@@ -202,7 +202,7 @@ with _`UpdateRule`_` = AdamUpdate`.
 | `double` | **`beta1`** | Exponential decay rate for the first moment estimates. | `0.9` |
 | `double` | **`beta2`** | Exponential decay rate for the weighted infinity norm estimates. | `0.999` |
 | `double` | **`eps`** | Value used to initialize the mean squared gradient parameter. | `1e-8` |
-| `size_t` | **`max_iterations`** | Maximum number of iterations allowed (0 means no limit). | `100000` |
+| `size_t` | **`maxIterations`** | Maximum number of iterations allowed (0 means no limit). | `100000` |


Nice catch! 👍

doc/optimizers.md

rcurtin · 2020-12-01T19:25:41Z

include/ensmallen_bits/demon_adam/demon_adam.hpp

+
+using DemonNadaMax = DemonAdamType<NadaMaxUpdate>;
+
+using DemonOptimisticAdam = DemonAdamType<OptimisticAdamUpdate>;


Maybe it's worth at least mentioning these typedefs in the DemonAdam and DemonSGD documentation?

Agreed, good idea.

rcurtin · 2020-12-01T19:29:29Z

include/ensmallen_bits/demon_adam/demon_adam_update.hpp

+    assert(momentumIterations != 0 && "The number of iterations before the "
+        "momentum will decay is zero, make sure the max iterations and "
+        "batch size parameter is set correctly. "
+        "Default: momentumIterations = maxIterations * batchSize.");


Shouldn't this be maxIterations / batchSize as the default? Otherwise the number of decay steps for momentum won't match the number of steps taken during optimization.

Nice catch!

include/ensmallen_bits/demon_sgd/demon_sgd_update.hpp

Co-authored-by: Ryan Curtin <ryan@ratml.org>

zoq · 2022-02-22T13:20:55Z

include/ensmallen_bits/demon_adam/demon_adam.hpp

+#define ENSMALLEN_DEMON_ADAM_DEMON_ADAM_HPP
+
+#include <ensmallen_bits/sgd/sgd.hpp>
+#include "demon_adam_update.hpp"


I'll update the includes today.

We may need to sort out which format to use (<xyz> vs "xyz").

Use of <...> caused problems within the context of RcppArmadillo (see for example RcppCore/RcppArmadillo#315, and related issues). The tldr is that for people that had two versions of Armadillo (one via RcppArmadillo, and one via plain Armadillo), there were clashes between the headers from the two versions. We settled on "xyz" to avoid this.

Within the context of ensmallen, rather than doing something like #include <ensmallen_bits/sgd/sgd.hpp>, perhaps we can use .. to specify one level back in the directory structure, ie. #include "../sgd/sgd.hpp". Alternatively, rather than having subdirectories, we have a flat layout in ensmallen_bits, ie. avoid use of further subdirectories. While the latter option may seem more messy, it has the advantage of all files being visible at the same time.

My only opinion on the format is that it's consistent. :) It sounds like "" is the right way to go here. How should we document this to ensure that it doesn't happen again? Style guide? Build check that greps for #include <ensmallen_bits/?

That also means, I should revert https://github.com/mlpack/ensmallen/pull/234/files/7776391f212b6c20a80c5a7fdb58785625736451#r810741863 and use "xyz" instead and adapt <ensmallen_bits/sgd/sgd.hpp> accordingly.

I'd like to keep the sub-directory structure, I think it's cleaner, and I don't mind to go one level back and forth. On the GitLab Armadillo repository page, the browser search function is unusable for me; the interface only lists the files in the viewport, so if I search for a file, I have to scroll down to the end, then I can search for a specific file.

…uite.

rcurtin

Everything looks good to me here. 👍

It seems like the sanity checks are incorrect or irrelevant---maybe we should filter out those messages?

rcurtin · 2022-04-03T03:49:57Z

include/ensmallen_bits/demon_adam/demon_adam_update.hpp

+#ifndef ENSMALLEN_DEMON_ADAM_DEMON_ADAM_UPDATE_HPP
+#define ENSMALLEN_DEMON_ADAM_DEMON_ADAM_UPDATE_HPP
+
+#include <assert.h>


I imagine this is already included by other headers? (Also I'm not sure it's needed. Correct me if I'm wrong.)

No assert in https://github.com/mlpack/ensmallen/blob/master/include/ensmallen.hpp.

mlpack-bot

Second approval provided automatically after 24 hours. 👍

zoq added 6 commits August 2, 2020 17:36

Add DemonSGD optimizer.

dfc5dd3

Add DemonSGD optimizer tests.

760f9b9

Add DemonAdam optimizer.

1124e63

Add DemonAdam optimizer tests.

006137e

Add DemonSGD and DemonAdam optimizer documentation.

42bd092

Add DemonSGD and DemonAdam optimizer to the history.

00d5304

zoq added c: optimizers s: needs review labels Aug 2, 2020

zoq added 3 commits August 2, 2020 17:47

Minor style fix (indentation).

584cfe1

Explicit type cast to avoid the loss of a fractional part.

a57ff2d

Explicit type cast to avoid the loss of a fractional part and minor s…

7af6407

…tyle fix.

zoq commented Aug 3, 2020

View reviewed changes

zoq added 3 commits August 8, 2020 21:07

Remove momentum iterations parameter.

4dd4cc4

Use number of iterations and batch size to intalize the momentum iter…

cd4a636

…ations.

Make sure the momentum iterations parameter is non-zero.

a64420b

UtR491 mentioned this pull request Aug 11, 2020

Adding MOEA/D as a multi-objective optimizer to ensmallen. #210

Closed

4 tasks

Merge branch 'master' into DemonSGD

c8f205c

mlpack-bot bot added the s: stale label Sep 28, 2020

mlpack-bot bot removed the s: stale label Sep 28, 2020

mlpack-bot bot added the s: stale label Oct 28, 2020

zoq added s: keep open and removed s: stale labels Oct 28, 2020

rcurtin reviewed Dec 1, 2020

View reviewed changes

zoq and others added 10 commits December 1, 2020 22:05

Minor spelling fix.

7d9ceb1

Co-authored-by: Ryan Curtin <ryan@ratml.org>

Minor spelling fix.

0265d8b

Co-authored-by: Ryan Curtin <ryan@ratml.org>

Minor spelling fix.

5e4a914

Co-authored-by: Ryan Curtin <ryan@ratml.org>

Merge branch 'master' into DemonSGD

6066fee

Reference Demon typedefs.

c2ec446

Report the correct maximum number of iterations.

ec7d984

Merge branch 'master' into DemonSGD

427d4b6

Add DemonSGD and DemonAdam optimizer documentation.

b9413f2

Update test suits to use updated test framework.

7bc1ee0

Merge branch 'master' into DemonSGD

f401c9e

zoq commented Feb 22, 2022

View reviewed changes

zoq added 4 commits February 23, 2022 20:39

Unify header inclusion format.

df69ffb

Use smaller step size for the first iteration to stabilize the test s…

8d68925

…uite.

Make demon test suite more stable.

5990bac

Merge branch 'master' into DemonSGD

a33dc77

rcurtin approved these changes Apr 3, 2022

View reviewed changes

mlpack-bot bot approved these changes Apr 4, 2022

View reviewed changes

zoq force-pushed the DemonSGD branch from 56fef97 to a33dc77 Compare April 5, 2022 03:06

zoq merged commit 1fd8d34 into mlpack:master Apr 6, 2022

zoq mentioned this pull request Apr 7, 2022

Release version 2.19.0: "Eight Ball Deluxe" #343

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DemonSGD & DemonAdam #211

DemonSGD & DemonAdam #211

zoq commented Aug 2, 2020

zoq left a comment

zoq commented Aug 8, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

mlpack-bot bot commented Sep 28, 2020

rcurtin commented Sep 28, 2020

mlpack-bot bot commented Oct 28, 2020

rcurtin left a comment

rcurtin Dec 1, 2020

rcurtin Dec 1, 2020

zoq Dec 1, 2020

rcurtin Dec 1, 2020

zoq Feb 15, 2022

zoq Feb 22, 2022

conradsnicta Feb 23, 2022 •

edited

Loading

rcurtin Feb 23, 2022

zoq Feb 23, 2022 •

edited

Loading

rcurtin left a comment

rcurtin Apr 3, 2022

zoq Apr 5, 2022

mlpack-bot bot left a comment


		using DemonNadaMax = DemonAdamType<NadaMaxUpdate>;

		using DemonOptimisticAdam = DemonAdamType<OptimisticAdamUpdate>;

DemonSGD & DemonAdam #211

DemonSGD & DemonAdam #211

Conversation

zoq commented Aug 2, 2020

zoq left a comment

Choose a reason for hiding this comment

zoq commented Aug 8, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

zoq commented Aug 9, 2020

mlpack-bot bot commented Sep 28, 2020

rcurtin commented Sep 28, 2020

mlpack-bot bot commented Oct 28, 2020

rcurtin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conradsnicta Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoq Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

rcurtin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlpack-bot bot left a comment

Choose a reason for hiding this comment

conradsnicta Feb 23, 2022 •

edited

Loading

zoq Feb 23, 2022 •

edited

Loading