Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Fix tensor type checking for pytorch 1.2 #1053

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

liyz15
Copy link

@liyz15 liyz15 commented Aug 20, 2019

Fix #725.
In pytorch 1.2, comparison operations return torch.bool instead of torch.uint8, which causes the error.
Ideally we should change all mask to torch.bool. Indexing with torch.uint8 also leads to infinite warning.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Aug 20, 2019
@zimenglan-sysu-512
Copy link
Contributor

nice!

@zimenglan-sysu-512
Copy link
Contributor

zimenglan-sysu-512 commented Aug 21, 2019

if u try to replace torch.uint8 with torch.bool, u should be aware of the evaluation in coco_eval.py, in which it needs to set like this:
rles = [
mask_util.encode(np.array(mask[0, :, :, np.newaxis], dtype=np.uint8, order="F"))[0]
for mask in masks
]
for more information, please see this.

@koenvandesande
Copy link

I ran into this one as well, ended up with the same fix. Please merge :)

@botcs
Copy link
Contributor

botcs commented Sep 9, 2019

Hi,

Thanks @liyz15 for the fix, I think many of us had this locally, but were too lazy to do a PR :)) so I really appreciate the effort.

@zimenglan-sysu-512 Thank you for pointing this out, I have spent a few hours trying to find out which part I messed up by converting all uint8 to bool.

Contacted @fmassa about PRs waiting for review, hopefully we could move the whole project to use pytorch 1.2 and rewrite the INSTALL.md accordingly soon.

@botcs
Copy link
Contributor

botcs commented Sep 16, 2019

Hi,

I've got permission to merge PRs, so we can start updating the lib to use PyTorch v1.2 (or 1.3?)
There are multiple spots in the source where torch.uint8 is used, and it would be definitely necessary to check if the MODEL_ZOO.md results can be reproduced precisely with higher PyTorch versions.

@ethanweber
Copy link

Thanks for the fix!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. contributions welcome enhancement New feature or request needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IndexError: list index out of range
6 participants