Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Bug] in operator over list-object slows down in hydra.utils.instatiate #1388

Closed
2 tasks done
S-aiueo32 opened this issue Feb 9, 2021 · 2 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@S-aiueo32
Copy link

🐛 Bug

Description

If through the hydra.utils.instantiate, the built-in in operator over list-objects slows down. It takes 200-times seconds.

Checklist

  • I checked on the latest version of Hydra
  • I created a minimal repro (See this for tips).

To reproduce

** Minimal Code/Config snippet to reproduce **

import random
import time
from contextlib import contextmanager
from typing import List

import hydra
from omegaconf import OmegaConf


@contextmanager
def timer(name: str) -> None:
    t0 = time.time()
    yield
    print(f'[{name}] done in {time.time() - t0:.03f} s')


class Filter:
    def __init__(self, min: int, max: int, n_samples: int, targets: List[int]) -> None:
        samples = [random.randint(min, max) for _ in range(n_samples)]
        # targets = set(targets)
        filtered = [s for s in samples if s in targets]


def app() -> None:

    config = OmegaConf.create({
        '_target_': '__main__.Filter',
        'min': 0,
        'max': 100,
        'n_samples': 100_000,
        'targets': [1, 2, 3]
    })

    kwargs = OmegaConf.to_container(config)
    kwargs.pop('_target_')

    with timer('Hydra instantiate'):
        _ = hydra.utils.instantiate(config)

    with timer('Basic instantiate'):
        _ = Filter(**kwargs)


if __name__ == '__main__':
    app()

** Stack trace/error message **

[Hydra instantiate] done in 16.938 s
[Basic instantiate] done in 0.084 s

Expected Behavior

If the comment-out for the conversion to set is removed, the execution time becomes almost the same.

[Hydra instantiate] done in 0.085 s
[Basic instantiate] done in 0.078 s

I think the performance over list-objects should be improved like this

System information

  • Hydra Version : 1.0.6 (also 1.1.0.dev3)
  • Python version : 3.8.6
  • Virtual environment type and version : pyenv+poetry
  • Operating system : ubuntu 18.04

Additional context

👍

@S-aiueo32 S-aiueo32 added the bug Something isn't working label Feb 9, 2021
@S-aiueo32 S-aiueo32 changed the title [Bug] in operator against list slows down in hydra.utils.instatiate [Bug] in operator over list-object slows down in hydra.utils.instatiate Feb 9, 2021
@omry
Copy link
Collaborator

omry commented Feb 9, 2021

OmegaConf features like interpolation are not free and does have some runtime cost.
As an example, this works:

cfg = OmegaConf.create(
    {
        "list": [1, "${a}"],
        "a": 10,
    }
)
assert 10 in cfg.list

While I am certain speed of various operators can be improved, the performance will never match native simpler containers.
You are welcome to try to optimize the __contains__ method in ListConfig and send a pull request to OmegaConf if you succeed. If you are interested file an issue against OmegaConf and I will give you some pointers.

Hydra 1.1 adds a _convert_ flag to instantiation which supports conversion of the OmegaConf containers to primitive containers.
That flag lets you use a simple list instead of the ListConfig, getting back the speed but losing the extra features.

See this.

@omry
Copy link
Collaborator

omry commented Feb 9, 2021

Opened omry/omegaconf#529.

Speed difference from simple list for contains is much more reasonable now after omry/omegaconf#530, but still bad for normal iteration. You are very welcome to dig into the benchmark there and try to improve.

@omry omry closed this as completed Feb 9, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants