Skip to content
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

Excessively long initialization time for Inception3 #1797

Closed
os-gabe opened this issue Jan 28, 2020 · 10 comments
Closed

Excessively long initialization time for Inception3 #1797

os-gabe opened this issue Jan 28, 2020 · 10 comments

Comments

@os-gabe
Copy link
Contributor

os-gabe commented Jan 28, 2020

Creating an instance of the Inception3 class via torchvision.models.inception_v3(pretrained=False) or torchvision.models.Inception3() takes several minutes to complete.

It appears that all of the time is being spent here here.

I believe this may be related to some change in scipy perhaps as I do not think this model used to take so long to instantiate.

@os-gabe
Copy link
Contributor Author

os-gabe commented Jan 28, 2020

I confirmed that rolling scipy from 1.4.1 back to 1.3.1 causes the issue to go away.

So this is not strictly a torchvision bug but it does mean that Inception3 becomes somewhat unusable for people who install the current scipy version from pip.

@fmassa
Copy link
Member

fmassa commented Jan 28, 2020

@os-gabe one alternative, given that we don't really train inception from scratch, would be to remove the custom initialization based on scipy.

That would be a breaking change though.

@os-gabe
Copy link
Contributor Author

os-gabe commented Jan 28, 2020

Here is the scipy issue for reference scipy/scipy#11299

@os-gabe
Copy link
Contributor Author

os-gabe commented Jan 28, 2020

@fmassa what if the Inception3 constructor took in an keyword argument to skip the random initialization? If the flag defaulted to False wouldn't this maintain backwards compatibility? This seems like a fairly nice solution in general since the random initialization is not needed when we just plan to load weights anyway.

@fmassa
Copy link
Member

fmassa commented Jan 28, 2020

Yeah, we could do something similar to what we do in VGG

def __init__(self, features, num_classes=1000, init_weights=True):

with init_weights. Can you send a PR for inception?

os-gabe added a commit to os-gabe/vision that referenced this issue Jan 29, 2020
@fmassa fmassa closed this as completed in 791c172 Jan 30, 2020
@fmassa
Copy link
Member

fmassa commented Jan 30, 2020

Fixed with #1832

@mingjunliu2
Copy link

Is this released?

@os-gabe
Copy link
Contributor Author

os-gabe commented Mar 21, 2020

There hasn't been a release since this change was merged. I'm curious to know when the next release is planned for.

@mingjunliu2
Copy link

I had very bad time to debug problem related to this issue.

@fmassa
Copy link
Member

fmassa commented Mar 23, 2020

Next release for torchvision will happen in a couple of weeks. But the underlying problem is with scipy, so the "fix" we use in torchvision is to skip the default initialization if the user specifies it.

fmassa pushed a commit to fmassa/vision-1 that referenced this issue Jul 8, 2020
facebook-github-bot pushed a commit that referenced this issue Jul 8, 2020
#2420)

Summary:
…(#1832)

Pull Request resolved: #2420

Reviewed By: zhangguanheng66

Differential Revision: D22432607

Pulled By: fmassa

fbshipit-source-id: e87cad1cc6015d05e413bc77cb0feda00d610919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants