Skip to content

Conversation

ydcjeff
Copy link
Contributor

@ydcjeff ydcjeff commented Dec 19, 2020

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 19, 2020

Thanks for the PR @ydcjeff . First, we have to fix our automatic docker image build check. It is WIP.
Then about the PR, I think a bit more solution is to install pillow-simd and remove all additional packages. Another solution is to build it like apex in a builder. I'll come back a bit later on this PR...
Another thing I was also thinking is that main/Dockerfile.base also ships torchvision from pytorch docker image and it would make sense to replace pillow-simd too.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 25, 2020

First, we have to fix our automatic docker image build check

@ydcjeff , This is done now. So we can proceed with this PR.

Let's try the following solution:

  • measure the size of docker image before changing Pillow -> Pillow-SIMD
  • as the last docker layer: install g++ and only requirements for building Pillow (not build-essential)
  • build Pillow-SIMD as usual
  • remove all installed debian packages: g++ etc and run apt autoremove
  • measure the enw size of docker image after changing Pillow -> Pillow-SIMD
  • report here

What do you think ?

@ydcjeff
Copy link
Contributor Author

ydcjeff commented Jan 16, 2021

@vfdev-5 since pillow is already installed in base docker files, I replaced pillow with pillow-simd in those docker files.
It seems I don't have permission to push to circleci.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 10, 2021

@ydcjeff sorry for delay on this PR, if you plan to continue on that, could you please do the following steps:

  • measure the size of docker image before changing Pillow -> Pillow-SIMD

  • edit dockerfile by adding g++ and building Pillow-SIMD as you did +
    remove all installed debian packages: g++ etc and run apt autoremove (currently missing)

  • measure the new size of docker image after changing Pillow -> Pillow-SIMD and report it here

@ydcjeff
Copy link
Contributor Author

ydcjeff commented Feb 13, 2021

@vfdev-5

  • old docker image size (pytorchignite/base): 9.97 GB [link]
  • new docker image size (pytorchignite/base): 6.27 GB [link]

[Dockerfile]

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 13, 2021

Thanks a lot @ydcjeff ! Just realized that pytorch finally released 1.7.1 docker image.

I checked that from my side and seems like the temporary hack to install pytorch 1.7.1 on 1.7.0 image costs space. Removing it and using now available 1.7.1 docker image then base image is

pytorchignite/base             latest                          d59f896e06f0        About a minute ago   6.12GB

I checked also if autoremove removes something

    apt-get -y install --no-install-recommends git && \
    apt-get autoremove -y && \    # <------ this one
    rm -rf /var/lib/apt/lists/*

and image size is still 6.12 GB.

To handle properly installing and removing of g++ we have to do that in the same docker layer:

# replace pillow with pillow-simd
RUN apt-get update && apt-get -y install --no-install-recommends g++ && \
    pip uninstall -y pillow && \
    CC="cc -mavx2" pip install --upgrade --no-cache-dir --force-reinstall pillow-simd && \
    apt-get remove -y g++ && \
    apt-get autoremove -y && \
    rm -rf /var/lib/apt/lists/*

Doing that we'll have

pytorchignite/base              latest                          bc9b095457ed        43 seconds ago      6.14GB

Anyway, I'll send a PR with removing the hack and then we merge this one then.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 14, 2021

@ydcjeff this way your docker image size will be still as it has g++. You should have 6.24Gb for base image. Please, do it I suggested above. Thanks

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ydcjeff !

@vfdev-5 vfdev-5 changed the base branch from master to pillow-simd February 14, 2021 12:43
@vfdev-5 vfdev-5 merged commit fdbc422 into pytorch:pillow-simd Feb 14, 2021
@ydcjeff ydcjeff deleted the docker/pillow-simd branch February 14, 2021 13:12
vfdev-5 added a commit that referenced this pull request Feb 14, 2021
* [docker] Pillow -> Pillow-SIMD (#1509)

* [docker] Pillow -> Pillow-SIMD

* replace pillow with pillow-simd in base docker files

* chore(docker): apt-get autoremove after pillow-simd installation

* apt-get install at once, autoremove g++

* install g++ in pillow installation layer

Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Fix g++ install issue

Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>
fco-dv pushed a commit to fco-dv/ignite that referenced this pull request Feb 15, 2021
* [docker] Pillow -> Pillow-SIMD (pytorch#1509)

* [docker] Pillow -> Pillow-SIMD

* replace pillow with pillow-simd in base docker files

* chore(docker): apt-get autoremove after pillow-simd installation

* apt-get install at once, autoremove g++

* install g++ in pillow installation layer

Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Fix g++ install issue

Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>
vfdev-5 added a commit that referenced this pull request Feb 21, 2021
* Recall/Precision metrics for ddp : average == false and multilabel == true

* For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5… (#1612)

* For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5.0 to v0.4.3

* Update ignite/contrib/metrics/regression/canberra_metric.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/contrib/metrics/regression/manhattan_distance.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/contrib/metrics/regression/r2_score.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/handlers/checkpoint.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* address PR comments

Co-authored-by: vfdev <vfdev.5@gmail.com>

* added TimeLimit handler with its test and doc (#1611)

* added TimeLimit handler with its test and doc

* fixed documentation

* fixed docstring and formatting

* flake8 fix trailing whitespace :)

* modified class logger , default value and tests

* changed rounding to nearest integer

* tests refactored , docs modified

* fixed default value , removed global logger

* fixing formatting

* Added versionadded

* added test for engine termination

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update handlers to use setup_logger (#1617)

* Fixes #1614
- Updated handlers EarlyStopping and TerminateOnNan
- Replaced `logging.getLogger` with `setup_logger` in the mentioned handlers

* Updated `TimeLimit` handler.
Replaced use of `logger.getLogger` with `setup_logger` from `ignite.utils`

Co-authored-by: Pradyumna Rahul K <pradyumnar@sahaj.ai>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Managing Deprecation using decorators (#1585)

* Starter code for managing deprecation

* Make functions deprecated using the `@deprecated` decorator
* Add arguments to the @deprecated decorator to customize it for each function

* Improve `@deprecated` decorator and add tests

* Replaced the `raise` keyword with added `warnings`
* Added tests several possibilities of the decorator usage

* Removing the test deprecation to check tests

* Add static typing, fix mypy errors

* Make `@deprecated` to raise Exceptions or Warning

* The `@deprecated` decorator will now always emit warning unless explicitly asked to raise an Exception

* Fix mypy errors

* Fix mypy errors (hopefully)

* Fix the test `test_deprecated_setup_any_logging`

* Change the test to work with the `@deprecated` decorator

* Change to snake_case, handle mypy ignores

* Improve Type Annotations

* Update common.py

* For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5… (#1612)

* For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5.0 to v0.4.3

* Update ignite/contrib/metrics/regression/canberra_metric.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/contrib/metrics/regression/manhattan_distance.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/contrib/metrics/regression/r2_score.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update ignite/handlers/checkpoint.py

Co-authored-by: vfdev <vfdev.5@gmail.com>

* address PR comments

Co-authored-by: vfdev <vfdev.5@gmail.com>

* `version` -> version

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: François COKELAER <francois.cokelaer@gmail.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Create documentation.md

* Distributed tests on Windows should be skipped until fixed. (#1620)

* modified CONTRIBUTING.md

* bash instead of sh

* Added Checkpoint.get_default_score_fn (#1621)

* Added Checkpoint.get_default_score_fn to simplify best_model_handler creation

* Added score_sign argument

* Updated docs

* Update about.rst

* Update pre-commit hooks and CONTRIBUTING.md (#1622)

* Change pre-commit config and CONTRIBUTING.md

- Update hook versions
- Remove seed-isort-config
- Add black profile to isort

* Fix files based on new pre-commit config

* Add meaningful exclusions to prettier

- Also update actions workflow files to match local pre-commit

* added requirements.txt and updated readme.md (#1624)

* added requirements.txt and updated readme.md

* Update examples/contrib/cifar10/README.md

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Update examples/contrib/cifar10/requirements.txt

Co-authored-by: vfdev <vfdev.5@gmail.com>

Co-authored-by: vfdev <vfdev.5@gmail.com>

* Replace relative paths with raw.githubusercontent (#1629)

* Updated cifar10 example (#1632)

* Updates for cifar10 example

* Updates for cifar10 example

* More updates

* Updated code

* Fixed code-formatting

* Fixed failling CI and typos for cifar10 examples (#1633)

* Updates for cifar10 example

* Updates for cifar10 example

* More updates

* Updated code

* Fixed code-formatting

* Fixed typo and failing CI

* Fixed hvd spawn fail and better synced qat code

* Removed temporary hack to install pth 1.7.1 (#1638)

- updated default pth image for gpu tests
- updated TORCH_CUDA_ARCH_LIST
- fixed /merge -> /head in trigger ci pipeline

* [docker] Pillow -> Pillow-SIMD (#1509) (#1639)

* [docker] Pillow -> Pillow-SIMD (#1509)

* [docker] Pillow -> Pillow-SIMD

* replace pillow with pillow-simd in base docker files

* chore(docker): apt-get autoremove after pillow-simd installation

* apt-get install at once, autoremove g++

* install g++ in pillow installation layer

Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Fix g++ install issue

Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Fix multinode tests script (#1631)

* fix run_multinode_tests_in_docker.sh : run tests with docker python version

* add missing modules

* build an image with test env and add 'nnodes' 'nproc_per_node' 'gpu' as parameters

* #1615 : change nproc_per_node default to 4

* #1615 : fix for gpu enabled tests / container rm step at the end of the script

* add xfail decorator for tests/ignite/engine/test_deterministic.py::test_multinode_distrib_cpu

* fix script gpu_options

* add default tol=1e-6 for _test_distrib_compute_on_criterion

* fix for "RuntimeError: trying to initialize the default process group twice!"

* tolerance for test_multinode_distrib_cpu case only

* fix assert None error

* autopep8 fix

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>
Co-authored-by: fco-dv <fco-dv@users.noreply.github.com>

* remove warning for average=False and is_multilabel=True

* update docstring and {precision, recall} tests according to test_multilabel_input_NCHW

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Ahmed Omar <40790298+ahmedo42@users.noreply.github.com>
Co-authored-by: Pradyumna Rahul <prkinformed@gmail.com>
Co-authored-by: Pradyumna Rahul K <pradyumnar@sahaj.ai>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>
Co-authored-by: Devanshu Shah <56106207+Devanshu24@users.noreply.github.com>
Co-authored-by: Debojyoti Chakraborty <debomastet335@gmail.com>
Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
Co-authored-by: fco-dv <fco-dv@users.noreply.github.com>
vfdev-5 added a commit that referenced this pull request Mar 3, 2021
* [docker] Pillow -> Pillow-SIMD (#1509)

* [docker] Pillow -> Pillow-SIMD

* replace pillow with pillow-simd in base docker files

* chore(docker): apt-get autoremove after pillow-simd installation

* apt-get install at once, autoremove g++

* install g++ in pillow installation layer

Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>

* Fix g++ install issue

Co-authored-by: Jeff Yang <ydcjeff@outlook.com>
Co-authored-by: Sylvain Desroziers <sylvain.desroziers@gmail.com>
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.

Install Pillow-SIMD to docker images

3 participants