Skip to content

Blob::offset() indexing#6021

Open
hkxIron wants to merge 1 commit intoBVLC:masterfrom
hkxIron:patch-1
Open

Blob::offset() indexing#6021
hkxIron wants to merge 1 commit intoBVLC:masterfrom
hkxIron:patch-1

Conversation

@hkxIron
Copy link

@hkxIron hkxIron commented Nov 1, 2017

Fix bug issue #5964 : change CHECK_LE to CHECK_LT in function offset.

Fix bug issue BVLC#5964 : change CHECK_LE to CHECK_LT in function offset.
@hkxIron
Copy link
Author

hkxIron commented Nov 1, 2017

Thanks.

@hkxIron
Copy link
Author

hkxIron commented Nov 2, 2017

@shaibagon , run unit test failed.
F1101 15:51:51.257525 12917 blob.hpp:158] Check failed: c < channels() (1 vs. 1)

@shaibagon
Copy link
Member

shaibagon commented Nov 2, 2017

@hkxIron It seems like pooling layer's CPU code uses offset() to compute a step size inside the blob, This step is taken at the end of a loop, so it might be the case that the step is computed, but when it falls outside the Blob it's the end of the loop and the memory is not accessed.
I guess this PR will have to deal/fix this as well.
I would try and pre-compute these steps in advance (i.e., calling offset(0,1) once outside the loop and saving the value and then re-using it instead of calling offset(0,1) multiple times.

This is why we have automatic tests: you can never know what a small change can affect :(

@Noiredd
Copy link
Member

Noiredd commented Feb 14, 2018

I would also add #792 for reference - I think that PR was more comprehensive - and #3420 for the conversation.

@Noiredd Noiredd added the bug label Feb 14, 2018
@Noiredd Noiredd changed the title Update blob.hpp for bug issue #5964 Blob::offset() indexing Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants