-
Notifications
You must be signed in to change notification settings - Fork 928
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
Update Dockerfile to add Catalyst #616
Conversation
Hi @Scitator, Can you add tests to prevent regression. Examples can be found under the Thank you |
Hi @rosbo So minimal test case is done ;) |
Hi @Scitator, If you can add a simple smoke test that exercise a common path of catalyst that would be great. Because we are installing 1000s of packages in this image, some dependencies may get reinstalled to a different version which may cause you package to fail inside the Kaggle image even though it works in your own CI (given the dependency may differ). Thank you |
@rosbo Would mnist test be enough? |
@rosbo Could you please check example now? |
Hmm...why do you think tests have failed? |
|
It seems like the installation probably failed. I will look at the build logs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logs seems to suggest the package was successfully installed:
Sep 26 22:03:20 Successfully installed catalyst-19.9.5 crc32c-1.7 safitty-1.2.5: Successfully installed catalyst-19.9.5.
The image it ran the tests against is gcr.io/kaggle-images/python:PR-616-pretest
. I will pull it and inspect it to see if the catalyst package is present.
criterion=criterion, | ||
optimizer=optimizer, | ||
loaders=loaders, | ||
logdir=logdir, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to call some sort of Close
method?
The tests shows a ResourceWarning:
unclosed file <_io.TextIOWrapper name='/working/logs/log.txt' mode='a' encoding='UTF-8'>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I found the issue.
The package is properly installed on the image:
$ pip show catalyst
Name: catalyst
Version: 19.9.5
Summary: Catalyst. High-level utils for PyTorch DL & RL research.
Home-page: https://github.com/catalyst-team/catalyst
Author: Sergey Kolesnikov
Author-email: [email protected]
License: Apache License 2.0
Location: /opt/conda/lib/python3.6/site-packages
Requires: opencv-python, tensorboardX, matplotlib, scikit-image, crc32c, pandas, PyYAML, torchvision, numpy, tqdm, scikit-learn, safitty, torch, imageio, plotly, seaborn
$ python
>>> import catalyst
>>> print(catalyst.__version__)
19.09.5
You only need to add the missing import statement (see my inline code comment).
@rosbo Looks like all checks have passed :) |
Hi, just want to add Catalyst - high-level PyTorch framework for reproducible and fast R&D.