fastNLP icon indicating copy to clipboard operation
fastNLP copied to clipboard

[bugfix] Bugfix embed_loader.py

Open LinyangHe opened this issue 5 years ago • 2 comments

Description:简要描述这次PR的内容 Bugfix embed_loader.py Main reason: 做出这次修改的原因 Some lines in the glove file do not contain an embedded word. There're only embedding vectors in these lines. For these lines, len(line) equals to 'emb_dim', rather than 'emb_dim + 1'. Just setting 'if len(line) > 2' won't work for all the lines.

Checklist 检查下面各项是否完成

Please feel free to remove inapplicable items for your PR.

  • [x] The PR title starts with [$CATEGORY] (例如[bugfix]修复bug,[new]添加新功能,[test]修改测试,[rm]删除旧代码)
  • [x] Changes are complete (i.e. I finished coding on this PR) 修改完成才提PR
  • [x] All changes have test coverage 修改的部分顺利通过测试。对于fastnlp/fastnlp/的修改,测试代码必须提供在fastnlp/test/
  • [x] Code is well-documented 注释写好,API文档会从注释中抽取
  • [x] To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change 修改导致例子或tutorial有变化,请找核心开发人员

Changes: 逐项描述修改的内容

  • modify 'if len(line) > 2' to 'if len(line) == emb_dim + 1'
  • add parameter 'emb_dim' in function '_load_pretrain()' and '_load_glove()'

Mention: 找人review你的PR @xuyige

LinyangHe avatar Jan 17 '19 10:01 LinyangHe

Codecov Report

Merging #128 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #128   +/-   ##
=====================================
  Coverage      68%    68%           
=====================================
  Files          90     90           
  Lines        6286   6286           
=====================================
  Hits         4275   4275           
  Misses       2011   2011
Impacted Files Coverage Δ
fastNLP/io/embed_loader.py 55.73% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3fa95b6...31c564c. Read the comment docs.

codecov-io avatar Jan 17 '19 10:01 codecov-io

Could you please provide a small piece of glove that cause the bug as an example at here? As we haven't met such issue when using Glove. Thanks.

choosewhatulike avatar Jan 19 '19 11:01 choosewhatulike