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

update itk requirement to >= 5.2 #1629

Closed
wyli opened this issue Feb 23, 2021 · 8 comments · Fixed by #2615
Closed

update itk requirement to >= 5.2 #1629

wyli opened this issue Feb 23, 2021 · 8 comments · Fixed by #2615
Labels
enhancement New feature or request Packaging WG: IO For the IO working group

Comments

@wyli
Copy link
Contributor

wyli commented Feb 23, 2021

@Nic-Ma Nic-Ma added the enhancement New feature or request label Mar 2, 2021
@wyli
Copy link
Contributor Author

wyli commented Apr 5, 2021

itk 5.2.0 has been released, it is causing some issue:
for example, to replicate it (with monai da3240f):

docker run --gpus all --rm -ti --ipc=host --net=host -v ~/Documents/MONAI:/opt/monai nvcr.io/nvidia/pytorch:20.03-py3
# within the docker container run:
cd /opt/monai
python -m pip install --upgrade pip wheel
python -m pip uninstall -y torch torchvision
python -m pip install torch==1.7.1 torchvision==0.8.2
python -m pip install -r requirements-dev.txt
python -m tests.test_distributed_sampler    # run tests/test_distributed_sampler.py

outcome:

/opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown
  len(cache))
/opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown
  len(cache))
./opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown
  len(cache))
/opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown
  len(cache))
.
----------------------------------------------------------------------
Ran 2 tests in 12.074s

OK

example log: https://github.com/Project-MONAI/MONAI/runs/2266945686?check_suite_focus=true
eventually the integration tests become very slow with UserWarning: semaphore_tracker:

I had a quick look, the same code works fine with itk 5.1.2. so I suspect it's an issue in the way MONAI using itk

any idea @thewtex ?

@wyli
Copy link
Contributor Author

wyli commented Apr 5, 2021

a smaller example to reproduce (python3 + linux + pytorch 1.7/1.8):

from torch import multiprocessing

try:
    multiprocessing.set_start_method("spawn")
except RuntimeError:
    pass

import itk
import torch

if __name__ == "__main__":
    for d in torch.utils.data.DataLoader([torch.tensor(i) for i in range(10)], num_workers=4):
        pass

this works fine with itk 5.1.2 but has the warnings with itk5.2.0

perhaps this global RLock is causing the issue https://github.com/InsightSoftwareConsortium/ITK/blob/6dfbc9f0314b5fd64d4b2ae392ae3cc6dda17677/Wrapping/Generators/Python/itk/support/lazy.py#L29

cc @hjmjohnson

@wyli wyli mentioned this issue Apr 5, 2021
1 task
@wyli wyli added the WG: IO For the IO working group label May 13, 2021
@wyli wyli added the Packaging label May 13, 2021
thewtex added a commit to thewtex/ITK that referenced this issue May 25, 2021
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
@thewtex
Copy link
Contributor

thewtex commented May 25, 2021

@wyli thanks for the detailed, reproducible report 🥇

I verified that this patch addresses the issue:

InsightSoftwareConsortium/ITK#2552

thewtex added a commit to thewtex/ITK that referenced this issue May 25, 2021
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
thewtex added a commit to thewtex/ITK that referenced this issue May 25, 2021
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.

We also add a threading lock in addition to a multiprocessing
lock.
thewtex added a commit to thewtex/ITK that referenced this issue May 25, 2021
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.
thewtex added a commit to thewtex/ITK that referenced this issue May 26, 2021
This addresses:

> /opt/conda/lib/python3.6/multiprocessing/semaphore_tracker.py:143: UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown len(cache))

Reported in Project-MONAI/MONAI#1629

Here we adopt the approach in tqdm:

  tqdm/tqdm#617

to address the similar issues with PyTorch:

  tqdm/tqdm#611

but for our use case. A global set of locks is added to the class on
first use.
@thewtex
Copy link
Contributor

thewtex commented May 28, 2021

@wyli this has been addressed in itk-5.2.0.post3.

I will help with the remaining issues based on this package in the coming month.

@wyli
Copy link
Contributor Author

wyli commented May 28, 2021

thanks @thewtex!

wyli added a commit to wyli/MONAI that referenced this issue May 28, 2021
@wyli wyli mentioned this issue May 28, 2021
7 tasks
@wyli
Copy link
Contributor Author

wyli commented Jun 2, 2021

it still gives the warning:

/opt/conda/lib/python3.8/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 33 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '

https://github.com/Project-MONAI/MONAI/runs/2728004520?check_suite_focus=true#step:7:2534
using 5.2.0.post3 PR here: #2274

@thewtex
Copy link
Contributor

thewtex commented Jun 18, 2021

@wyli thanks for the report. We will keep pushing on this! Tracked in InsightSoftwareConsortium/ITK#2603

wyli added a commit to wyli/MONAI that referenced this issue Jul 15, 2021
@thewtex
Copy link
Contributor

thewtex commented Aug 20, 2021

Following-up: itk-5.2.1 contains additional improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Packaging WG: IO For the IO working group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants