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

[Question] replacing instatiate(ObjConf) with already instatiated object #1722

Closed
robogast opened this issue Jul 20, 2021 · 3 comments
Closed

Comments

@robogast
Copy link

robogast commented Jul 20, 2021

Since OmegaConf doesn't allow non-primitive types, we need to specify object's configurations instead.
However, we might end up in a situation where one of the sub-objects is already initialized, and we wish to simply use this object as-is.
Naively replacing the object results in either a ValidationError (omegaconf.errors.ValidationError: Invalid type assigned) or ImportError: Encountered error: Invalid type assigned (as expected), see the example below (save file as test.py).

One option is to manually create the object ourselves, extracting the configuration options as provided, but this forgoes the power of the more general instantiate function.
Better would be a flag to indicate that the object attached to the config is already constructed, and to just ignore (the type of) this arg, but for now I don't see how this is possible at the moment.

from dataclasses import dataclass

from hydra.utils import instantiate
from omegaconf import MISSING

def foo():
    pass

def bar():
    pass

@dataclass
class ObjAConf:
  _target_: str = 'test.ObjA'

@dataclass
class ObjBConf:
  _target_: str = 'test.ObjB'
  ObjA: ObjAConf = MISSING

class ObjA:
  def __init__(self):
    foo()

class ObjB:
  def __init__(self, ObjA):
    self.obj_a = ObjA
    bar()


obj_a = instantiate(ObjAConf)
obj_b = instantiate(ObjBConf, ObjA=obj_a)

# ImportError: Encountered error: `Invalid type assigned: ObjA is not a subclass of ObjAConf. value: <datamodules.tests.test_config.ObjA object at 0x150f10795ca0>
#     full_key: ObjA
#     object_type=ObjBConf` when loading module 'datamodules.tests.test_config.ObjA'
@omry
Copy link
Collaborator

omry commented Jul 20, 2021

This is wrong:

@dataclass
class ObjBConf:
  _target_: str = 'test.ObjB'
  ObjA: ObjAConf = MISSING

the desired type for ObjA is not ObjAConf, but ObjA (the class, the fact you are using the same name for the field make things more confusing).
You can either remove ObjA: ObjAConf = MISSING completely, or make the annotated type Any.

@robogast
Copy link
Author

robogast commented Jul 22, 2021

But how do you deal with the relation (and types) between ObjConf <-> Obj then?

For example, the following two ways of initialisation result in equivalent objects (edited from the previous example following your comment about confusing variable names):

from dataclasses import dataclass

from hydra.utils import instantiate


class ObjA:
  def __init__(self, a: int):
    self.a = a

class ObjB:
  def __init__(self, obj_a: ObjA, b: int):
    self.obj_a = obj_a
    self.b = b

@dataclass
class ObjAConf:
  _target_: str = 'test.ObjA'
  a: int = 5

@dataclass
class ObjBConf:
  _target_: str = 'test.ObjB'
  obj_a: ObjAConf = ObjAConf
  b: int = 7
  
# two different ways to obtain the same objects
obj_b_inst = instantiate(ObjBConf)
obj_b_init = ObjB(ObjA(a=5), b=7)

print(f"From instantiate: obj_b.b={obj_b_inst.b}, obj_b.obj_a.a={obj_b_inst.obj_a.a}")
print(f"From initialise:  obj_b.b={obj_b_init.b}, obj_b.obj_a.a={obj_b_init.obj_a.a}")

# From instantiate: obj_b.b=7, obj_b.obj_a.a=5
# From initialise:  obj_b.b=7, obj_b.obj_a.a=5

From the ObjBConf's perspective, its obj_a needs to be of type ObjAConf, otherwise instantiate doesn't recursively initialise all objects...(!)

So my original question still somewhat stands, making the type of ObjBConf.obj_a Any feels more like a band-aid solution, as we do have knowledge of what the actual type should be.
Maybe a fitting solution would be that ObjBConf.obj_a could be of type Union[ObjA, ObjAConf], but I believe that is currently not allowed?

@omry
Copy link
Collaborator

omry commented Jul 23, 2021

As I said:

You can either remove ObjA: ObjAConf = MISSING completely, or make the annotated type Any.

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

No branches or pull requests

2 participants