Keras compatability -- easy addition?

JianGoForIt / YellowFin

auto-tuning momentum SGD optimizer

Apache License 2.0

422 stars 93 forks source link

Keras compatability -- easy addition? #1

Closed jmhessel closed 7 years ago

jmhessel commented 7 years ago

Hi! Thanks for posting this code. I thought that I would give YF a try as a drop-in optimizer. Currently, I am using Keras, and I was able to modify your code to run on Keras models by doing the following:

Adding a compute_gradients standalone method
Adding a few checks for gradients being None in apply_gradients and after_apply
Wrapping the YFOptimizer object in a Keras TFOptimizer

However, while it runs and my loss goes down -- I am not 100% I did everything properly. Do you think you might consider adding this support?

Zehaos commented 7 years ago

Hi @jmhessel,

I'm now going through what you have done, I wanna to know whether you have trained a reasonable model？ and what dataset are you training on?

Best, Zehao

Zehaos commented 7 years ago

@jmhessel I have validated it on mnist dataset, got 97.64% accuracy on test set.

JianGoForIt commented 7 years ago

Hi @Zehaos @jmhessel,

Thanks for helping us validating and integrating.

@jmhessel For the three points, I think we can add here the first two points. For the last one, it would be the best practice to request Keras team to integrate. We could try to reach out. What do you think?

Cheers,

Jian

JianGoForIt commented 7 years ago

@jmhessel Could you please do a pull request with your modification for the first two points?

Thanks.

jmhessel commented 7 years ago

@JianGoForIt Sure -- I can do a quick PR (I actually didn't even bother saving the code because the modifications were so minor, but it will be easy to re-do once I get the chance in the next few days). With respect to point 3, I don't think you'll need to get in contact with the keras folks -- when I say that the YFOptimizer is wrapped in the TFOptimizer, I mean that it is done programmatically (i.e., it's not a subclass if TFOptimizer or anything, you simply write opt=TFOptimizer(YFOptimizer()) which is pretty clean, as is.

JianGoForIt commented 7 years ago

@jmhessel Ha, got your point right this time. That sounds great! I will merge your PR as soon as it is done.

JianGoForIt commented 7 years ago

Keras support added using commits for pull request here https://github.com/JianGoForIt/YellowFin/commit/0949dfe2e674e1b83bdd06bbc21cf9b2d920ae1f