search

asyml / texar-pytorch

Integrating the Best of TF into PyTorch, for Machine Learning, Natural Language Processing, and Text Generation. This is part of the CASL project: http://casl-project.ai/
https://asyml.io
Apache License 2.0
745 stars 117 forks source link

Fix bug in rnn #252

Closed Codle closed 5 years ago

Codle commented 5 years ago

When using rnn without sequence_length, the new tensor created for sequence_length don't have same device as inputs. It will cause the torch.utils.mask_sequences arguments have different device.

huzecong commented 5 years ago

Thanks for catching this, and thanks for creating a pull request! I think there are also other tensor creation ops that needs fixing, for instance the one on line 282. Could you also fix those?

codecov[bot] commented 5 years ago

Codecov Report

Merging #252 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   83.05%   83.05%   +<.01%     
==========================================
  Files         195      195              
  Lines       15290    15293       +3     
==========================================
+ Hits        12699    12702       +3     
  Misses       2591     2591
Impacted Files Coverage Δ
texar/torch/utils/rnn.py 97.18% <100%> (ø) :arrow_up:
texar/torch/custom/distributions.py 40% <0%> (ø) :arrow_up:
texar/torch/custom/__init__.py 100% <0%> (ø) :arrow_up:
texar/torch/custom/activation.py 100% <0%> (ø) :arrow_up:
texar/torch/custom/initializers.py 57.69% <0%> (+1.69%) :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 53d8ead...8cbdacf. Read the comment docs.

Codle commented 5 years ago

OK, I will check other creation ops tomorrow.

huzecong commented 5 years ago

Please also make sure that the code passes our integration tests, which includes formatting checks. Now it fails because you have a trailing whitespace.