Learning rate by JimClarke5 · Pull Request #106 · tensorflow/java · GitHub

This PR requires PR "Initial checkin of Keras Optimzers and helper classes" to be merged first.

Added changeable learning rate to Optimizers. This was done by adding a Placeholder for the learning rate, a Tensor to track the actual learning rate, and adding a Map to map the Placeholder to the Tensor that can be used to "feed" the runner.
Test Sessions were modified to accept a "FeedDict" Map to popullate the feed() of the runner.

…lder into each Optimizer. Also, added to each Optimizer a corresponding Tensor that holds the value of the learning rate, and added a feed dictionary that maps the placeholder to the Tensor, so that it can be fed into the runner when running or evaluating. When setLearning rate is called the learning rate tensor and the feed dictionary are updated.

A couple of quick observations as I start reading this code.

This pattern is very uncomfortable to me: DataType being omniscient about TType and dispatching on a string NAME. What's our motivation? If we think it's the best pattern for this situation, perhaps we could document why?

Never mind, in this context -- I see in my local diff that this delta is unrelated to this PR. I'll raise this as an issue.

-> RHO_DEFAULT, EPSILON_DEFAULT

Never mind, in this context.

Sync

Can we have both of these constructors call into the Optimizer(Graph,float,String) one?

Being that Optimizer is abstract, we really only need one constructor, protected Optimizer(Graph graph, String name, float learningRate) . Of course, we would have to handle a null name, with something like:
this.tf = Ops.create(graph).withName(name == null? getOptimizerName() : name);

CTORS have been changed

Why'd the Javadoc go away?

I am not sure what happened. I had a local copy that I saved and it was there, so will add it back in.

Update pushed

Where'd the javadoc go?

I have added it back in. Update pushed

Everything seems to have grown a this reference. I don't think that's particularly necessary in these methods, as the argument could be newLearningRate rather than learningRate and then there is no aliasing.

I do this out of habit. I can easily change it as you suggest.

changed setLearningRate to setLearningRate(float newLearningRate), removed spurious this..

Update pushed

…ewLearningRate), eliminated spurious "this."

typo - "earning"

OK

…raph graph, String name, float learningRate)"", change all the subclass ctors to use this one.

…feedMap to null.

Add Operand<TFloat32> learningRateOperand as an option for learning rate.

Just a few snake_case variable names in the tests that need converting to camelCase, and then I'll merge this in.

Please switch the python style variable names to camelCase.

Switch python style name to camelCase.

This PR requires some rework due to #174, Type Refactor, Should we close this PR and open a new one later? Also, this PR is closely related to callbacks, so I suggest we revisit this one after callbacks is done. My current plan is #180, Metrics Phase1, followed by Metrics Phase2, then Model/Layers Phase 1, then callbacks, then this PR can be revisited.

This PR requires some rework due to #174, Type Refactor, Should we close this PR and open a new one later? Also, this PR is closely related to callbacks, so I suggest we revisit this one after callbacks is done. My current plan is #180, Metrics Phase1, followed by Metrics Phase2, then Model/Layers Phase 1, then callbacks, then this PR can be revisited.

Whatever you think is easiest is fine by me.

I am closing for now, and will reopen after we get further along on Model.

JimClarke5 added 10 commits August 30, 2020 18:42

Add support for hanling feed dicts when evaluating or printing Operands.

0afdb9c

Add tests for changing learning rates

9f10da9

Moved Optimizers to Keras. Added support for chanign learning rate.

0cc4012

Moved Optimizers to Keras. Added support for chanign learning rate.

d8fab04

Reformatted code

dddc297

Reformatted code

cb8104c

Reformatted code

15189b4

Remove premature commit

eb2c48e

deansher suggested changes Sep 17, 2020

View reviewed changes

Merge pull request #2 from JimClarke5/master

a2b5d7b

Sync

Craigacp reviewed Sep 19, 2020

View reviewed changes

Added JavaDoc back in, changed setLearningRate() to setLearningRate(n…

d5edd35

…ewLearningRate), eliminated spurious "this."

Craigacp reviewed Sep 21, 2020

View reviewed changes

JimClarke5 added 4 commits September 21, 2020 15:00

Change Optimizer to only have one constructor, "protected Optimizer(G…

2f57c1d

…raph graph, String name, float learningRate)"", change all the subclass ctors to use this one.

Fixed close() routine to free up closed tensor in feedMap by setting …

e9e2b24

…feedMap to null.

Fix javadoc for references to Default values,

62ff85c

Add Operand<TFloat32> learningRateOperand as an option for learning rate.

Added Operand<TFloat32> learningRateOperand test case for learning rate.

ca1395e

Craigacp requested changes Sep 30, 2020

View reviewed changes

karllessard mentioned this pull request Oct 14, 2020

Add Losses #129

Merged

karllessard mentioned this pull request Feb 11, 2021

Add Constraints #197

Closed

JimClarke5 closed this May 28, 2021

+                 * @param graph the TensorFlow Graph
+                 * @param name the name for this Optimizer (defaults to 'Adadelta')
+                 * @param learningRate the learning rate
+                 */
                 public AdaDelta(Graph graph, String name, float learningRate) {
                   this(graph, name, learningRate, 0.95f, 1e-8f);

+                 * @param learningRate the learning rate.
                  */
-                protected Optimizer(Graph graph, String name) {
+                protected Optimizer(Graph graph, float learningRate) {

+                 * @param learningRate the learning rate
+                 */
+                public final void setLearningRate(float learningRate) {
+                  if (this.learningRatePlaceholder == null) {

+                            new RMSProp(session.getGraph(), learningRate, decay, momentum, epsilon, centered)) {
+                      Ops tf = session.getTF();
+                      session.setEpsilon(1e-2f);
+                      float[] var0_init = {1.0F, 2.0F};

                 private FloatNdArray calculateParam(
-                    FloatNdArray param, float lrT, FloatNdArray m, FloatNdArray v, float epsilon) {
-                  //  param - lrT * mT / (np.sqrt(vT) + epsilon)
+                    FloatNdArray param, float lr_t, FloatNdArray m, FloatNdArray v, float epsilon) {

+                }
+                /** Returns true if this data type represents a floating point type */
+                public boolean isFloating() {

                   return variable.op().name() + "-" + slotName;
                 }
-                /**

                   }
                 }
-                /**

                  * Sets the learning rate
                  *
-                 * @param learningRate the learning rate
+                 * @param newLearningRate the new earning rate

Learning rate by JimClarke5 · Pull Request #106 · tensorflow/java · GitHub

Uh oh!

Learning rate - #106

Learning rate#106JimClarke5 wants to merge 16 commits intotensorflow:mastertensorflow/java:masterfrom JimClarke5:learning_rateJimClarke5/java:learning_rateCopy head branch name to clipboard

Conversation

JimClarke5 commented Sep 1, 2020

Uh oh!

deansher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Craigacp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JimClarke5 commented Jan 3, 2021

Uh oh!

Craigacp commented Jan 3, 2021

Uh oh!

JimClarke5 commented May 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Learning rate#106
JimClarke5 wants to merge 16 commits into
tensorflow:masterfrom
JimClarke5:learning_rate