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

Add ESIM model #169

Merged
merged 10 commits into from Jan 29, 2019

Conversation

Projects
None yet
2 participants
@Victor0118
Copy link
Member

commented Dec 4, 2018

Reference:
paper: Enhanced {LSTM} for Natural Language Inference
code: https://github.com/lanwuwei/SPM_toolkit/tree/master/ESIM

@likicode @daemon Could you take a look at this PR?

Victor0118 added some commits Dec 4, 2018

@Victor0118

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2018

ESIM without tree after tuning on SICK dataset

==> esim_sick_1e-4_0.001_0.3_0.2.log <==
INFO - pearson_r spearman_r mse KL-divergence loss
INFO - test 0.878273 0.823042214423 0.25375571846961975 0.551383518848454

ESIM-Tree after tuning on SICK dataset

==> esim_tree_sick_1e-4_0.001_0.3_0.2.log <==
INFO - pearson_r spearman_r mse KL-divergence loss
INFO - test 0.878069 0.820418404008 0.2360289841890335 0.4703939018613406

@daemon daemon self-requested a review Dec 16, 2018



if __name__ == '__main__':
parser = argparse.ArgumentParser(description='PyTorch implementation of Multi-Perspective CNN')

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

description is good to include.


device = torch.device(f'cuda:{args.device}' if torch.cuda.is_available() and args.device >= 0 else 'cpu')

random.seed(args.seed)

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

Did we fold this part into a common module? E.g., import common.utils; common.utils.set_all_seeds(x)?

import torch
import numpy as np
import torch.nn as nn
from esim.util import *

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

PEP8 import order.

from datetime import datetime
from torch.nn.utils.rnn import pack_padded_sequence, pad_packed_sequence

def ortho_weight(ndim):

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

Does torch.nn.init have orthogonal initialization?

u, s, v = np.linalg.svd(W)
return u.astype('float32')

def norm_weight(nin, nout=None, scale=0.01, ortho=True):

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

torch has this, right?

h = o * F.tanh(c)
return c, h

class LSTM(nn.Module):

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

Isn't this the same as nn.LSTM?

for step in range(x.size(0)):
input=x[step] # #sample x dim_emb
step_c, step_h=self.TreeCell(input, h, c)
h=x_mask[step][:,None] * step_h + (1. - x_mask[step])[:,None] * h

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

Why not use the faster fused nn.LSTM with pack_padded_sequence?


#tokenizer = nltk.tokenize.TreebankWordTokenizer()

def pearson(x,y):

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

SciPy has a Pearson's r routine, right?

This comment has been minimized.

Copy link
@Victor0118

Victor0118 Dec 17, 2018

Author Member

Oh. Will try that. Thanks!

if test_data_label[sortedi]==True:
truepos+=1
elif test_data_label[sortedi]==False:
falsepos+=1

This comment has been minimized.

Copy link
@daemon

daemon Dec 16, 2018

Member

This file is the only one that uses tabs; should use 4 spaces.

Show resolved Hide resolved esim/util.py Outdated
@daemon

This comment has been minimized.

Copy link
Member

commented Dec 16, 2018

Mostly nitpicks. What's the reason for reimplementing an LSTM?

@Victor0118

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2019

@daemon Sorry. Just saw it. I think I just follow the raw implementation: https://github.com/lanwuwei/SPM_toolkit/tree/master/ESIM. No idea why the author reimplemented it.

@daemon

daemon approved these changes Jan 29, 2019

@Victor0118 Victor0118 merged commit aec0826 into castorini:master Jan 29, 2019

@Victor0118 Victor0118 deleted the Victor0118:add-esim branch Jan 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.