Skip to content

add torch.script tests to convert_image_dtype #2313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Jun 11, 2020

Addresses #2078 (comment)

@pmeier
Copy link
Collaborator Author

pmeier commented Jun 11, 2020

Test failure seems related. @fmassa Could you have a look if I misunderstood the usage of torch.jit.script?

@fmassa
Copy link
Member

fmassa commented Jun 11, 2020

I think the test failures are because torch.jit.script expects a function, but you are passing the output of the function (which is a tensor)

@pmeier
Copy link
Collaborator Author

pmeier commented Jun 12, 2020

I think the test failures are because torch.jit.script expects a function, but you are passing the output of the function (which is a tensor)

Oh dang, that should have been obvious. Sorry, I was tired when I wrote it.

@fmassa
Copy link
Member

fmassa commented Jun 12, 2020

I think test failures might be either due to a breakage in PyTorch (cc @eellison does it ring a bell for the Faster R-CNN and Mask R-CNN tests?) or you need to rebase on top of master (which I think it's the case, just confirming)

@pmeier
Copy link
Collaborator Author

pmeier commented Jun 12, 2020

Will try the rebase.

I've seen errors such as this:

RuntimeError: 
Tried to access nonexistent attribute or method 'is_floating_point' of type 'int'.:

I don't think this will be fixed by a rebase. Is it possible that I'm not allowed to use that attribute in torch.script?

Edit: This branch is based on c2e8a00, which is the HEAD of master at the time of writing this.

@fmassa
Copy link
Member

fmassa commented Jun 12, 2020

@pmeier I see, it's an issue with torchscript (it currently doesn't have dtype mapped to torchscript internally).

For now, the workaround would probably be to do

img.dtype in [torch.float16, torch.float32, torch.float64]

until torchscript supports it.

But then, we will have another issue afterwards, which is that torchscript doesn't support finfo and iinfo

I think for now it's better to wait until torchscript adds better support for dtype, so let's hold on with this PR for now. :-/

cc @eellison

@pmeier
Copy link
Collaborator Author

pmeier commented Jun 12, 2020

I've pushed the last commit before I saw your comment. You can ignore it for noe and I will revert it when we move forward with this PR.

@pmeier
Copy link
Collaborator Author

pmeier commented Oct 5, 2020

Superseded by #2485.

@pmeier pmeier closed this Oct 5, 2020
@pmeier pmeier deleted the convert_dtype_script branch October 21, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants