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

Parse callables (functions, methods) similarly to objects #5852

Closed
ibro45 opened this issue Jan 14, 2023 · 10 comments · Fixed by #5854
Closed

Parse callables (functions, methods) similarly to objects #5852

ibro45 opened this issue Jan 14, 2023 · 10 comments · Fixed by #5854

Comments

@ibro45
Copy link
Contributor

ibro45 commented Jan 14, 2023

Is your feature request related to a problem? Please describe.
Support for functions/methods in config by making Monai call them instead of attempting to instantiate them.

calculate:
    _target_: "math.isclose"
    a: 0.001
    b: 0.002

Describe the solution you'd like
A solution like Hydra's (https://hydra.cc/docs/advanced/instantiate_objects/overview/) that can both instantiate or call would be perfect.

Describe alternatives you've considered
None

Additional context
While you could do calculate: "$math.isclose(0.001, 0.002)", it does not allow argument overriding from CLI.

@wyli
Copy link
Contributor

wyli commented Jan 14, 2023

thanks for reporting, it's possible to use @ to refer to an object, so that it supports overriding:

from monai.bundle import ConfigParser

config = {"import statements": "$import math",
          "calculate": "$math.isclose(@a, @b)",
          "a": 0.001,
          "b": 0.005}
parser = ConfigParser(config)
print(parser.get_parsed_content("calculate"))  # False
parser["b"] = 0.001
print(parser.get_parsed_content("calculate"))  # True

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 14, 2023

Hi, yes, thank you, but you still have to refer the arguments manually in"$math.isclose(@a, @b)". With _target_ and instantiation that's not necessary.

Do you think you're interested in adding support for calling functions/methods in the same fashion as object instantiation?

@wyli
Copy link
Contributor

wyli commented Jan 14, 2023

I took another look, actually it's a supported use case, but there's bug in the code. I'm create a pull request to fix it.

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 14, 2023

Awesome, thank you so much. I love what you did; I switched from OmegaConf/Hydra, Monai Bundle Config solves quite a few problems I had there, and the error messages are way better. Super simple to integrate it too. Looking forward to learning more about Bundle Specification!

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 14, 2023

Hi, so I just installed monai from your branch and tried this:

config.yaml

imports: $import torch

forward:
    _target_: "@model.forward"
    input: $torch.rand(1, 3, 256, 256)

model:
    _target_: monai.networks.nets.resnet.resnet18
    spatial_dim: 2

Command:
python -m monai.bundle run forward --config_file config.yaml

And it raises:

Traceback (most recent call last):
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/config_item.py", line 286, in instantiate
    return instantiate(modname, **args)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/utils/module.py", line 232, in instantiate
    raise ModuleNotFoundError(f"Cannot locate class or function path: '{path}'.")
ModuleNotFoundError: Cannot locate class or function path: '@model.forward'.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/__main__.py", line 20, in <module>
    fire.Fire()
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/fire/core.py", line 141, in Fire
    component_trace = _Fire(component, args, parsed_flag_args, context, name)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/fire/core.py", line 466, in _Fire
    component, remaining_args = _CallAndUpdateTrace(
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/fire/core.py", line 681, in _CallAndUpdateTrace
    component = fn(*varargs, **kwargs)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/scripts.py", line 744, in run
    return [parser.get_parsed_content(i, lazy=True, eval_expr=True, instantiate=True) for i in ensure_tuple(runner_id_)]
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/scripts.py", line 744, in <listcomp>
    return [parser.get_parsed_content(i, lazy=True, eval_expr=True, instantiate=True) for i in ensure_tuple(runner_id_)]
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/config_parser.py", line 287, in get_parsed_content
    return self.ref_resolver.get_resolved_content(id=id, **kwargs)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/reference_resolver.py", line 188, in get_resolved_content
    return self._resolve_one_item(id=id, **kwargs)
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/reference_resolver.py", line 166, in _resolve_one_item
    self.resolved_content[id] = item.instantiate() if kwargs.get("instantiate", True) else item
  File "/home/ibro/anaconda3/envs/lighter/lib/python3.9/site-packages/monai/bundle/config_item.py", line 288, in instantiate
    raise RuntimeError(f"Failed to instantiate {self}.") from e
RuntimeError: Failed to instantiate {'_target_': '@model.forward', 'input': tensor([[[[0.3608, 0.3325, 0.4329,  ..., 0.0211, 0.4916, 0.6840],
          [0.0417, 0.7033, 0.3991,  ..., 0.3597, 0.1550, 0.3324],
          [0.6108, 0.8000, 0.8778,  ..., 0.7984, 0.9651, 0.8256],
          ...,
          [0.7286, 0.7389, 0.3762,  ..., 0.5201, 0.3182, 0.0289],
          [0.4781, 0.3437, 0.1659,  ..., 0.1252, 0.5990, 0.7619],
          [0.0707, 0.7495, 0.2894,  ..., 0.3977, 0.8704, 0.3102]],

         [[0.5390, 0.2731, 0.1640,  ..., 0.6009, 0.2121, 0.9602],
          [0.8667, 0.7630, 0.8089,  ..., 0.0607, 0.2541, 0.7719],
          [0.8204, 0.3814, 0.4954,  ..., 0.6606, 0.3937, 0.6057],
          ...,
          [0.8998, 0.0648, 0.3001,  ..., 0.5004, 0.6388, 0.1619],
          [0.6452, 0.4225, 0.1998,  ..., 0.4859, 0.4119, 0.0660],
          [0.2361, 0.5676, 0.9900,  ..., 0.6868, 0.3248, 0.9527]],

         [[0.6602, 0.6643, 0.8554,  ..., 0.7943, 0.4970, 0.1568],
          [0.2356, 0.6178, 0.7380,  ..., 0.8589, 0.4696, 0.2227],
          [0.5740, 0.3209, 0.6339,  ..., 0.3270, 0.3660, 0.3420],
          ...,
          [0.7360, 0.1710, 0.9990,  ..., 0.1324, 0.6394, 0.5160],
          [0.0786, 0.9697, 0.0510,  ..., 0.3566, 0.4310, 0.2636],
          [0.0938, 0.4502, 0.2579,  ..., 0.5274, 0.5545, 0.3955]]]])}.

Am I doing something wrong, or is referencing a method of an object from the config not supported?

@wyli
Copy link
Contributor

wyli commented Jan 14, 2023

that's a good question, at the moment the usage would be

imports: $import torch

x: "$torch.rand(1, 3, 256, 256)"
forward: "$@model().forward(@x)"

model:
    _target_: monai.networks.nets.resnet.resnet18
    pretrained: False
    spatial_dims: 2

if you want to delay the forward call, it's also possible to add the partial wrapper:

imports:
    - $import torch
    - $import functools

x: "$torch.rand(1, 3, 256, 256)"
forward: "$functools.partial(@model().forward, @x)"

model:
    _target_: monai.networks.nets.resnet.resnet18
    pretrained: False
    spatial_dims: 2

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 14, 2023

Alright, thanks! Do you think that's something that the bundle config will support?

@wyli
Copy link
Contributor

wyli commented Jan 14, 2023

sure, it seems that is doable with minor code changes, I'm creating a pull request and getting more comments from @Nic-Ma @ericspod.

@ericspod
Copy link
Member

The way that we do searching when referring to something by name should perhaps be revisited. Currently our code traverses all known libraries looking for names, this a problem if multiple definitions use the same name in different modules as well as also forcing a lot of things to get loaded that don't need to be. The solution to your specific use case above works but there's a number of extra syntax that we've introduce with the "$@" sigils that isn't obvious. I feel we need a bit of a smarter name searching system that will look at some target like "X.Y.Z" and search for "X" first in currently known objects and imported modules, then for "X.Y" if not found, etc. It would then resolve the rest of the name looking for definitions or members of definitions, so a target "model.forward" should find the object "model" and that "forward" is a callable member and not try to instantiate it.

@wyli
Copy link
Contributor

wyli commented Jan 14, 2023

I've updated #5854 for this.
Previously the check of _target_ was too restrictive:

target = config.get("_target_")
if not isinstance(target, str):
raise ValueError("must provide a string for the `_target_` of component to instantiate.")

please see the new test cases in PR #5854 for the newly supported cases.

wyli added a commit that referenced this issue Jan 17, 2023
Signed-off-by: Wenqi Li <wenqil@nvidia.com>

Fixes #5852

### Description
1. the following partial instantiate doesn't work
```py
config = {"import statements": "$import math", 
          "calc": {"_target_": "math.isclose", "a": 0.001, "b": 0.001}}
print(ConfigParser(config).calc())
```
with an error message:
```
Component to instantiate must represent a valid class or function, but got math.isclose.
Traceback (most recent call last):
  File "test.py", line 4, in <module>
    print(ConfigParser(config).calc())
TypeError: isclose() missing required argument 'a' (pos 1)
```

because `math.isclose` is a builtin type but not a function:

```py
import inspect
inspect.isfunction(math.isclose)  # False
inspect.isbuiltin(math.isclose)  # True
```

the `partial` should support `callable` including builtin functions


2. also this PR supports `_target_` of reference object's methods such
as partial init:
```py
"forward": {"_target_": "$@model().forward", "x": "$torch.rand(1, 3, 256, 256)"}
```

### 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).
- [x] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: Wenqi Li <wenqil@nvidia.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 a pull request may close this issue.

3 participants