Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

[SCRIPT] Add ESIM for text matching #689

Merged
merged 5 commits into from
Jul 4, 2019
Merged

[SCRIPT] Add ESIM for text matching #689

merged 5 commits into from
Jul 4, 2019

Conversation

vanewu
Copy link
Contributor

@vanewu vanewu commented May 2, 2019

Description

Build an Enhancing LSTM model for Natural Language Inference.
The model does not currently contain the TreeLSTM mentioned in the original paper. Training scripts and tests are in progress.

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@vanewu vanewu requested a review from szha as a code owner May 2, 2019 16:38
@szha szha requested a review from hhexiy May 2, 2019 17:14
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
scripts/esim/esim.py Outdated Show resolved Hide resolved
self.lstm_encoder2 = rnn.LSTM(nhiddens, bidirectional=True)

self.fc_encoder = nn.HybridSequential()
self.fc_encoder.add(nn.BatchNorm(axis=-1),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experiment using a standard 1 layer FFN is good enough. The current fc_encoder gives NaN results somehow.

Copy link
Contributor Author

@vanewu vanewu May 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hhexiy The reason for this is that I previously mistyped the softmax axis of weight2. Many bugs have been fixed in the new submission based on your comments and original papers. I will do a full test on the data mentioned in the paper later. Thank you very much.

@vanewu
Copy link
Contributor Author

vanewu commented May 4, 2019

@hhexiy
Thanks for the comment. The model code is okay on a competition task. But it has not been run on public data. Testing and calibration work is in progress. I will check and test it according to your proposal, and ping you after the work is completed.

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

❗ No coverage uploaded for pull request head (ESIM@7bcd4ba). Click here to learn what that means.
The diff coverage is n/a.

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #689 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #689   +/-   ##
=======================================
  Coverage   90.38%   90.38%           
=======================================
  Files          66       66           
  Lines        6378     6378           
=======================================
  Hits         5765     5765           
  Misses        613      613

scripts/esim/esim.py Outdated Show resolved Hide resolved
@mli
Copy link
Member

mli commented Jun 23, 2019

Job PR-689/2 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-689/2/index.html

vanewu and others added 2 commits June 30, 2019 03:56
@mli
Copy link
Member

mli commented Jun 30, 2019

Job PR-689/3 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-689/3/index.html

@mli
Copy link
Member

mli commented Jun 30, 2019

Job PR-689/4 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-689/4/index.html

@szha szha requested a review from fierceX June 30, 2019 19:45
@mli
Copy link
Member

mli commented Jul 2, 2019

Job PR-689/5 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-689/5/index.html

@mli
Copy link
Member

mli commented Jul 2, 2019

Job PR-689/6 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-689/6/index.html

@szha szha changed the title [WIP] [SCRIPT] Add ESIM for text matching [SCRIPT] Add ESIM for text matching Jul 2, 2019
@szha szha merged commit edded0b into dmlc:master Jul 4, 2019
@szha
Copy link
Member

szha commented Jul 4, 2019

Some items left:

  • report performance and upload log to dmlc/web-data
  • update model zoo page to include the description of the model, the command, and link to the log

@eric-haibin-lin
Copy link
Member

@szha move the remaining items to github issue to track ?

@szha szha mentioned this pull request Jul 7, 2019
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants