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

SqeezeNet meta data for create_cnn function #1152

Merged
merged 6 commits into from Jan 13, 2019

Conversation

devforfu
Copy link

Based on forum's discussion, the PR makes an attempt to bring the support of SqeezeNet architecture into the library. Now you're able to use the architecture like ResNets:

learn = create_cnn(data, models.squeezenet1_1)

However, I've chosen the layers groups split quite arbitrary (splitting model on maxpool layers) so would be glad to see your comments if there is a better configuration.

@sgugger
Copy link
Contributor

sgugger commented Nov 13, 2018

Thanks!
Concerning your tests, please leave the full training only for one integration test with the simple CNN. Tests are also run on CPU and we can't have too many that are to slow (so only one training per application).

@sgugger
Copy link
Contributor

sgugger commented Nov 13, 2018

Please amend your PR to be able to merge with the latest big refactor. If it's easiest, close this one and reopen another.

@devforfu
Copy link
Author

Ok, sure, will do.

@devforfu
Copy link
Author

@sgugger Removed all training from the tests except one place with simple_cnn and merged with the most recent master. Now should be better.

@devforfu
Copy link
Author

Seems there is a bug in tests, going to fix soon.

@devforfu
Copy link
Author

devforfu commented Nov 20, 2018

@sgugger Seems like there is a timeout issue, however, I don't train any model in the tests I've added. Do you think it is worth to change something in the tests to decrease CI execution time?

@jph00 jph00 closed this Nov 30, 2018
@jph00 jph00 reopened this Nov 30, 2018
fastai/vision/learner.py Outdated Show resolved Hide resolved
tests/test_vision_train.py Outdated Show resolved Hide resolved
@devforfu
Copy link
Author

I've tried to address the issues. I think now the create_cnn should look better. How do you think, should we change anything else? Are these changes are still relevant to the library? Also, I've included the following test:

@pytest.mark.slow
@pytest.mark.parametrize('arch', [models.resnet18, models.squeezenet1_1])
def test_models_meta(mnist_tiny, arch, zero_image):
    learn = create_cnn(mnist_tiny, arch, metrics=[accuracy, error_rate])
    pred = learn.predict(zero_image)
    assert pred is not None

Note the last line. I am not really sure what to assert here so I am just making sure that the output is not None. The main goal of this test is to make sure that architectures are instantiated correctly. Actually, the test has passed if we just don't get any exception so it could be assert True as well.

@sgugger
Copy link
Contributor

sgugger commented Jan 13, 2019

Seems good, thanks!

@sgugger sgugger merged commit cab8e54 into fastai:master Jan 13, 2019
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.

None yet

3 participants