Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LAMB optimizer parses regex patterns improperly #1530

Closed
jarednielsen opened this issue Apr 1, 2020 · 1 comment
Closed

LAMB optimizer parses regex patterns improperly #1530

jarednielsen opened this issue Apr 1, 2020 · 1 comment
Labels
bug Something isn't working optimizers

Comments

@jarednielsen
Copy link
Contributor

jarednielsen commented Apr 1, 2020

The LAMB optimizer declares that its exclude_from_weight_decay argument should take in a comma-separated string of regex patterns. However, the code expects a list of regex patterns and instead iterates through each character in the string. Thus nearly every call to _do_use_weight_decay() returns False.

I attempted to pass in a list to circumvent this bug, but this leads to a typeguard error. So there's no easy way around this in the meantime. A similar bug exists for exclude_from_layer_adaption.

Two proposed fixes, and I'm happy to contribute either:

  • Change the desired datatype from Optional[str] to List[str]. This would be preferred, and match the style of other implementations in the TensorFlow repo. See here for a list of examples. My PR is open at Fix LAMB optimizer regex parsing #1532.
  • Add .split(',') to exclude_from_weight_decay and exclude_from_layer_adaption in the constructor.
@gabrieldemarmiesse
Copy link
Member

This has been fixed and ported to the 0.8.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working optimizers
Projects
None yet
Development

No branches or pull requests

3 participants