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

MONAI bundle instantiate fails when _target_ contains path attribute #5865

Closed
surajpaib opened this issue Jan 17, 2023 · 2 comments · Fixed by #5866
Closed

MONAI bundle instantiate fails when _target_ contains path attribute #5865

surajpaib opened this issue Jan 17, 2023 · 2 comments · Fixed by #5866

Comments

@surajpaib
Copy link
Contributor

Describe the bug
MONAI bundle instantiate fails when _target_ contains path attribute
The following error is thrown,

To Reproduce
I created a dataset.py file with the following

class MyDataset:
    def __init__(self, path=None) -> None:
        self.path = path
        print(path)

and run the following code which throws an error


    config = {
        "dataset": {
            "_target_": "dataset.MyDataset",
            "path": "test.csv"
        }

    }
    run("dataset", **config)

Error message:

Traceback (most recent call last):
  File "/home/suraj/anaconda3/envs/lightningssl/lib/python3.8/site-packages/monai/bundle/config_item.py", line 283, in instantiate
    return instantiate(modname, **args)
TypeError: instantiate() got multiple values for argument 'path'

Expected behavior
The dataset object should be instantiated.

Additional context
Seems like the issue is here:

def instantiate(path: str, **kwargs):

where the instantiate uses has a path attribute itself, causing the duplication when path is passed in kwargs as well.

Since path is a very common attribute used in datasets, it might be prudent to change the name of this argument to a non-conflicting name

Environment

Ensuring you use the relevant python executable, please paste the output of:

================================
Printing MONAI config...
================================
MONAI version: 1.1.0
Numpy version: 1.23.3
Pytorch version: 1.12.1+cu102
MONAI flags: HAS_EXT = False, USE_COMPILED = False, USE_META_DICT = False
MONAI rev id: a2ec3752f54bfc3b40e7952234fbeb5452ed63e3
MONAI __file__: /home/suraj/anaconda3/envs/lightningssl/lib/python3.8/site-packages/monai/__init__.py

Optional dependencies:
Pytorch Ignite version: NOT INSTALLED or UNKNOWN VERSION.
Nibabel version: NOT INSTALLED or UNKNOWN VERSION.
scikit-image version: NOT INSTALLED or UNKNOWN VERSION.
Pillow version: 9.2.0
Tensorboard version: 2.11.0
gdown version: NOT INSTALLED or UNKNOWN VERSION.
TorchVision version: 0.13.1+cu102
tqdm version: 4.64.1
lmdb version: NOT INSTALLED or UNKNOWN VERSION.
psutil version: 5.9.3
pandas version: 1.5.0
einops version: NOT INSTALLED or UNKNOWN VERSION.
transformers version: NOT INSTALLED or UNKNOWN VERSION.
mlflow version: NOT INSTALLED or UNKNOWN VERSION.
pynrrd version: NOT INSTALLED or UNKNOWN VERSION.

For details about installing the optional dependencies, please visit:
    https://docs.monai.io/en/latest/installation.html#installing-the-recommended-dependencies


================================
Printing system config...
================================
System: Linux
Linux version: Ubuntu 20.04.4 LTS
Platform: Linux-5.13.0-40-generic-x86_64-with-glibc2.10
Processor: x86_64
Machine: x86_64
Python version: 3.8.13
Process name: python
Command: ['python', '-c', 'import monai; monai.config.print_debug_info()']
Num physical CPUs: 12
Num logical CPUs: 12
Num usable CPUs: 12
CPU usage (%): [15.9, 16.2, 6.0, 9.1, 4.5, 8.9, 2.3, 2.2, 0.8, 3.0, 2.3, 100.0]
CPU freq. (MHz): 908
Load avg. in last 1, 5, 15 mins (%): [7.6, 8.9, 11.3]
Disk usage (%): 82.3
Avg. sensor temp. (Celsius): UNKNOWN for given OS
Total physical memory (GB): 250.6
Available memory (GB): 92.9
Used memory (GB): 5.8

================================
Printing GPU config...
================================
Num GPUs: 4
Has CUDA: True
CUDA version: 10.2
cuDNN enabled: True
cuDNN version: 7605
Current device: 0
Library compiled for CUDA architectures: ['sm_37', 'sm_50', 'sm_60', 'sm_70']
GPU 0 Name: Quadro RTX 8000
GPU 0 Is integrated: False
GPU 0 Is multi GPU board: False
GPU 0 Multi processor count: 72
GPU 0 Total memory (GB): 47.5
GPU 0 CUDA capability (maj.min): 7.5
GPU 1 Name: Quadro RTX 8000
GPU 1 Is integrated: False
GPU 1 Is multi GPU board: False
GPU 1 Multi processor count: 72
GPU 1 Total memory (GB): 47.5
GPU 1 CUDA capability (maj.min): 7.5
GPU 2 Name: Quadro RTX 8000
GPU 2 Is integrated: False
GPU 2 Is multi GPU board: False
GPU 2 Multi processor count: 72
GPU 2 Total memory (GB): 47.5
GPU 2 CUDA capability (maj.min): 7.5
GPU 3 Name: Quadro RTX 8000
GPU 3 Is integrated: False
GPU 3 Is multi GPU board: False
GPU 3 Multi processor count: 72
GPU 3 Total memory (GB): 47.5
GPU 3 CUDA capability (maj.min): 7.5
@wyli
Copy link
Contributor

wyli commented Jan 17, 2023

thanks for reporting, it makes sense to use a less common argument, perhaps _path or __path..

@surajpaib
Copy link
Contributor Author

I agree. I'm happy to contribute to the change as well!

wyli pushed a commit that referenced this issue Jan 18, 2023
Fixes #5865 

### Description

A few sentences describing the changes proposed in this pull request.

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: Suraj Pai <bspai@bwh.harvard.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants