-
Notifications
You must be signed in to change notification settings - Fork 56
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
Tweak conv-rnn model #75
Conversation
daemon
commented
Oct 29, 2017
- Fix misplaced zero_grad()
- Tweak model hyperparams and optimization algorithm
- Fix misplaced zero_grad() - Tweak model hyperparams and optimization algorithm
conv_rnn/test.py
Outdated
@@ -17,19 +18,19 @@ def main(): | |||
parser.add_argument("--gpu_number", default=0, type=int) | |||
args = parser.parse_args() | |||
|
|||
model.set_seed(5, no_cuda=args.no_cuda) | |||
data_loader = data.SSTDataLoader(args.data_dir) | |||
model.set_seed(3, no_cuda=args.no_cuda) |
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.
make seed configurable?
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.
Actually the seed is useless in test.py, I'll remove it later.
@@ -34,8 +34,6 @@ def __init__(self, word_model, **config): | |||
else: | |||
raise ValueError("RNN type must be one of LSTM or GRU") | |||
self.conv = nn.Conv2d(1, n_fmaps, (1, self.hidden_size * 2)) | |||
if dropout: |
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.
does this hurt performance? if we are removing it why are we adding --dropout_prob
to train.py
?
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.
Paper says no dropout for SST1/2.