fix bug 'vector iterator + offset out of range' in nnet-common #2594
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danpovey
reviewed
Aug 6, 2018
src/nnet3/nnet-common.cc
Outdated
| // check 'n2 > index_vector.size()' first, | ||
| // otherwise, 'end - n2' will be less than 'index_vector.begin()' and | ||
| // cause error "vector iterator + offset out of range" in STL vector. | ||
| if (n2 > index_vector.size() || iter >= end - n2) |
Contributor
There was a problem hiding this comment.
make 'size' a variable (declare it next to 'ans') as it's inefficient to keep using size() in a loop.
Contributor
Author
There was a problem hiding this comment.
updated! Define 'len' as 'index_vector.size()'. Pls review again. Thanks!
qindazhu
force-pushed
the
fix-bound-bug-of-nnet-common
branch
from
b1ff84f to
1df9cc1
Compare
danpovey
reviewed
Aug 8, 2018
src/nnet3/nnet-common.cc
Outdated
| ans += iter->x * 89809; | ||
| // check 'n2 > index_vector.size()' first, | ||
| // otherwise, 'end - n2' will be less than 'index_vector.begin()' and | ||
| // cause error "vector iterator + offset out of range" in STL vector. |
Contributor
There was a problem hiding this comment.
Please make sure this code is correctly indented (and make sure you use spaces not tabs).
Also, you can change the comment to the following:
// The following if-statement was introduced in order to fix an
// out-of-range iterator problem on Windows.
danpovey
reviewed
Aug 8, 2018
src/nnet3/nnet-common.cc
Outdated
| std::vector<Index>::const_iterator iter = index_vector.begin(), | ||
| end = index_vector.end(), med = end; | ||
| if (med > iter + n1) | ||
| if (n1 <= len) |
Contributor
There was a problem hiding this comment.
.. also I believe this should be if n1 < len, not <=.
Contributor
Author
There was a problem hiding this comment.
updated again. Also checked indentation and spaces. With another question:
why when med == end == iter + n1, we would like to do for (; iter != med; ++iter) instead of skip this? This will make the hash better?
qindazhu
force-pushed
the
fix-bound-bug-of-nnet-common
branch
3 times, most recently
from
bf2f017 to
43eeb9a
Compare
qindazhu
force-pushed
the
fix-bound-bug-of-nnet-common
branch
from
43eeb9a to
68d847d
Compare
xiaohui-zhang
pushed a commit
to xiaohui-zhang/kaldi
that referenced
this pull request
Aug 11, 2018
xiaohui-zhang
pushed a commit
to xiaohui-zhang/kaldi
that referenced
this pull request
Aug 11, 2018
…ng/limit_arpa_unk_history.py [src] Correct usage message of acc-lda (kaldi-asr#2598) [scripts] RNNLM script fix: to accept successive spaces in configs (etc.) kaldi-asr#2595 (kaldi-asr#2597) [scripts] Slight cleanup in lmrescore_rnnlm_lat.sh (kaldi-asr#2554) [src] Fix Windows out-of-range iterator issue for nnet3 (kaldi-asr#2594) [src] Update Windows installation instructions (kaldi-asr#2607) [egs] Fix to LibriSpeech download script [affects 2nd run] (kaldi-asr#2611) [src] Change RNNLM test program to clean up temporary file (kaldi-asr#2610) added a script to scale arcs which output <unk> in HCLG.fst fix fix updating results and cleaning up scripts, and fixed a bug in utils/lang/limit_arpa_unk_history.py [scripts] Fix to script usage message (kaldi-asr#2601) [src] Correct usage message of acc-lda (kaldi-asr#2598) [scripts] RNNLM script fix: to accept successive spaces in configs (etc.) kaldi-asr#2595 (kaldi-asr#2597) [scripts] Slight cleanup in lmrescore_rnnlm_lat.sh (kaldi-asr#2554) [src] Fix Windows out-of-range iterator issue for nnet3 (kaldi-asr#2594) [src] Update Windows installation instructions (kaldi-asr#2607) [egs] Fix to LibriSpeech download script [affects 2nd run] (kaldi-asr#2611) [src] Change RNNLM test program to clean up temporary file (kaldi-asr#2610)
dpriver
pushed a commit
to dpriver/kaldi
that referenced
this pull request
Sep 13, 2018
Skaiste
pushed a commit
to Skaiste/idlak
that referenced
this pull request
Sep 26, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.