-
Notifications
You must be signed in to change notification settings - Fork 3
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
Opts cleaning #61
Opts cleaning #61
Conversation
@shaoxiongji this could impact docs. |
smoketested with hydra config on 1 GPU during test. Seems to be working properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. A few of the maybes can be removed.
@@ -537,10 +350,12 @@ def model_opts(parser): | |||
group.add( | |||
'--transformer_ff', '-transformer_ff', type=int, default=2048, help='Size of hidden transformer feed-forward' | |||
) | |||
# TODO is this actually in use? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is passed around in the code. Not sure if functional, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaving is as is for now, ideally #56 would remove that
closes #60