Implementing the Adamax optimizer operator #4538
Conversation
paddle/operators/adamax_op.h
Outdated
| float beta_1 = ctx.Attr<float>("beta_1"); | ||
| float beta_2 = ctx.Attr<float>("beta_2"); | ||
| float epsilon = ctx.Attr<float>("epsilon"); | ||
| int t = ctx.Attr<int>("time_step"); |
There was a problem hiding this comment.
The timestep should not be an attribute. It should be an input of Adamax. That input type could be int.
There was a problem hiding this comment.
Thank you for the feedback. I will change this.
|
|
||
|
|
||
| class TestAdamaxOp1(OpTest): | ||
| def setUp(self): |
There was a problem hiding this comment.
You provided two TestAdamaxOp functions and commented that the second one is for testing default attributes. I think it would be helpful to also add a comment for TestAdamaxOp1 explaining its purpose. Also, I didn't find any differences between the two test functions. If the first function is to test explicit attributes, you should change the attribute values.
There was a problem hiding this comment.
Thank you. I forgot to remove the attributes from the second one . Will change this.
| } | ||
|
|
||
| def test_check_output(self): | ||
| self.check_output() |
There was a problem hiding this comment.
The test of this kind of operator(Optimizer with state) should be more complex because we have accumulated state. The state will change when running, so the test code should run multiple times to check if the state is right.
There was a problem hiding this comment.
Fixed in af36e75
Fixes #4515