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

[Feature Request] Consider supporting singleton pattern in recursive instantiation #1393

Open
omry opened this issue Feb 11, 2021 · 12 comments

Comments

@omry
Copy link
Collaborator

omry commented Feb 11, 2021

The following config can in theory be used to instantiate one object

container:
  _target_: ...
  object1:
    _target_: ...
    _singleton_: true

  object2: ${.object1}
c = instantiate(cfg.container)
assert c.object1 is c.object2

This will will probably require changes in OmegaConf.

@omry omry added the enhancement Enhanvement request label Feb 11, 2021
@omry omry changed the title [Feature Request] support singleton pattern in recursive instantiation [Feature Request] Consider supporting singleton pattern in recursive instantiation Feb 11, 2021
@omry omry added this to the Hydra 1.2.0 milestone Feb 18, 2021
@rsokl
Copy link
Contributor

rsokl commented Feb 21, 2022

This would be very powerful. It would effectively enable Hydra to be used for dependency injection (but orchestrated via configs).

@aliutkus

This comment was marked as outdated.

@aliutkus
Copy link

aliutkus commented Jun 3, 2022

As a note, I would like this to work:

misc:
  a:
     _target_: mymodule.A
     _singleton_: true
     foo: 1

b: 
   _target_: mymodule.B
   bar: 2
   a: ${misc.a}

c:
   _target_: mymodule.C
   baz: 3
   a: ${misc.a}

i.e., the reference would not need to happen inside the same object to be instantiated. This may be very useful for cases where the same object (say some datapipeline) is to be used by models or other things, while keeping some structure to the config

@martinj96

This comment was marked as outdated.

@Jasha10

This comment was marked as outdated.

@npuichigo

This comment was marked as outdated.

@Ubadub
Copy link

Ubadub commented May 9, 2023

Hi, I noticed that this was initially part of the Hydra 1.3.0 milestone but was removed. It seems like a very useful feature to have. Are there any updates on when we can expect this feature?

@npuichigo

This comment was marked as outdated.

@yuvalkirstain

This comment was marked as outdated.

@Jasha10
Copy link
Collaborator

Jasha10 commented Dec 25, 2023

I'll comment to record one potential issue I see with the proposed API.

It looks like calling resolve before instantiate will result in object1 and object2 being different objects.

from omegaconf import OmegaConf

cfgA = OmegaConf.create("""
object1:
 _target_: MyTarget
 _singleton_: true
object2: ${object1}
""")

cfgB = cfgA.copy()
OmegaConf.resolve(cfgB)

assert OmegaConf.to_yaml(cfgB) == """\
object1:
  _target_: MyTarget
  _singleton_: true
object2:
  _target_: MyTarget
  _singleton_: true
"""

Concretely, how would Hydra know that instantiate(cfgB) should have object1 and object2 be the same? Most operations in Hydra/OmegaConf respect the invariant that operating on a resolved config should give the same results as operating on an unresolved config.

@yuvalkirstain
Copy link

@Jasha10 thank you for the response. Just to stress out the issue, in most image generation/video generation projects we instantiate objects that use GPU and other expensive resources, and use them multiple times. Without this feature we need to manually pass these objects. This workaround breaks the clean flow that hydra usually enables 😢

@marios1861
Copy link

Is there any progress on this? The lack of this feature breaks the free config-based pattern that hydra enables for deep learning applications, as already stated by @yuvalkirstain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests