added optimizer registry

[jackd]

Added optimizer registration, similar to other registration methods. This allows users to add new optimizers in their t2t_usr_dir without having to manipulate tensor2tensor code directly.

EDIT: points below resolved
Error checking number of args in registered function is a bit gross (inspect doesn't play nicely with functools.partial), but unsure of a better way of handling this.

I'm not convinced the way of handling optimizers in tf.contrib.layers.OPTIMIZER_CLS_NAMES is optimal, but I don't have a better way to bind loop variables to function calls.

[afrozenator]

This is really neat @jackd -- many thanks! Will merge this in now.

Just FYI @rsepassi just a few days ago wrote a generic registry ee64f6f we'll port this (and maybe other registries) over to that abstraction -- but many thanks again!

[jackd]

@afrozenator cheers. I was considering abstracting away some of the registry details myself, glad I wasn't the only one.

I'm a little confused as to the point of the new implementation though - _GENERIC_REGISTRIES/create_registry/_reset don't seem be used anywhere except tests? _reset loops over hard-coded registers rather than all registered registries? There's also no way of registering registries with custom error checking (like those already implemented)?

_OPTIMIZERS register is looking a bit lonely down the bottom too (and so I'm not surprised it was overlooked in _reset).

In short, this isn't how I'd have refactored it and it looks like a mash up of competing ideas, but there may well be use cases this is trying to serve that I'm not appreciating. Happy to put in a separate PR, just don't want to step on anyone's toes.

[rsepassi]

Definitely welcome a PR on cleaning this stuff up. Do you want to think about how you’d design (or modify the just added one) a generic registry system and send over a doc or just update this thread? We want something simple and flexible. The new generic registry seemed like a good starting point but it sounds like it’s too limiting. We can work towards something better with your help.

…

On Thu, Jan 24, 2019 at 4:15 PM Dominic Jack ***@***.***> wrote: @afrozenator <https://github.com/afrozenator> cheers. I was considering abstracting away some of the registry details myself, glad I wasn't the only one. I'm a little confused as to the point of the new implementation though - _GENERIC_REGISTRIES/create_registry/_reset don't seem be used anywhere except tests? _reset loops over hard-coded registers rather than all registered registries? There's also no way of registering registries with custom error checking (like those already implemented)? _OPTIMIZERS register is looking a bit lonely down the bottom too (and so I'm not surprised it was overlooked in _reset). In short, this isn't how I'd have refactored it and it looks like a mash up of competing ideas, but there may well be use cases this is trying to serve that I'm not appreciating. Happy to put in a separate PR, just don't want to step on anyone's toes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1401 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEGW2e-o1tEsiHxr4Mp767uZgc_Lbnnks5vGkyFgaJpZM4aNpy8> .

[jackd]

@rsepassi So I started drafting a plan but it got a bit out of hand so I just went ahead and made a pull request. Happy to discuss/make changes, but probably best to continue conversation over there :)