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

Conversation

@leezu
Copy link
Contributor

@leezu leezu commented Nov 13, 2019

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

  • Fix TransformerXL LayerNorm eps. Change from 1e-12 to 1e-5

Comments

We previously asserted that our implementation computes the same result as the pytorch-transformers implementation. However, that implementation was wrong up until 1.2.0 release in which their implementation was fixed unintentionally / by accident in huggingface/transformers#1089.

This also updates the comparison scripts which broke to API breakage in the packages that we compare to (renaming / reordering of arguments etc.).

Also updates gelu to approx_gelu in the XLNet pretrained model configuration, which should have been done in #988.

cc @dmlc/gluon-nlp-team
@zburning FYI

@leezu leezu requested a review from a team as a code owner November 13, 2019 09:25
@codecov
Copy link

codecov bot commented Nov 13, 2019

Codecov Report

Merging #1005 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@ ## master #1005 +/- ## ========================================== + Coverage 89.93% 89.95% +0.01%  ========================================== Files 67 67 Lines 6340 6340 ========================================== + Hits 5702 5703 +1  + Misses 638 637 -1
Impacted Files Coverage Δ
src/gluonnlp/model/transformer.py 91.63% <0%> (+0.32%) ⬆️
@leezu leezu changed the title [Fix] TransformerXL LayerNorm eps and XLNet pretrained model config [Bugfix] TransformerXL LayerNorm eps and XLNet pretrained model config Nov 13, 2019
@mli
Copy link
Member

mli commented Nov 13, 2019

@leezu leezu added the release focus Progress focus for release label Nov 15, 2019
@mli
Copy link
Member

mli commented Nov 15, 2019

@leezu leezu force-pushed the fixtransformerxllayernormeps branch from 01de29d to d3b9e1d Compare November 18, 2019 07:43
@mli
Copy link
Member

mli commented Nov 18, 2019

np.unravel_index argument names changed in v1.16.0. scripts/language_model/conversion_utils/compare_transformerxl_pytorch_gluon_model.py requires v1.16.0 due to making use of the updated argument names. This was previously not correctly declared.
@leezu leezu force-pushed the fixtransformerxllayernormeps branch from d3b9e1d to 2de2a99 Compare November 20, 2019 08:22
@mli
Copy link
Member

mli commented Nov 20, 2019

@leezu leezu merged commit b2f4e7b into dmlc:master Nov 28, 2019
@leezu leezu deleted the fixtransformerxllayernormeps branch November 28, 2019 04:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release focus Progress focus for release

3 participants