omry / omegaconf

Flexible Python configuration system. The last one you will ever need.
BSD 3-Clause "New" or "Revised" License
1.94k stars 106 forks source link

Failure to merge interpolation into structured config #1020

Open Jasha10 opened 1 year ago

Jasha10 commented 1 year ago

Describe the bug As described in https://github.com/facebookresearch/hydra/issues/2416, merging a config with an interpolation key into a structured config can fail with an InterpolationKeyError or a ValidationError.

To Reproduce There are two failure modes identified in https://github.com/facebookresearch/hydra/issues/2416.

Mode 1: merging an interpolation into an optional None value
# omegaconf_failing.py
from dataclasses import dataclass
from typing import Optional
from omegaconf import OmegaConf

@dataclass
class TableConfig:
    rows: int = 1

@dataclass
class DatabaseConfig:
    table_cfg: TableConfig = TableConfig()

@dataclass
class ModelConfig:
    data_source: Optional[TableConfig] = None

@dataclass
class ServerConfig:
    db: DatabaseConfig
    model: ModelConfig

cfg = OmegaConf.merge(
    ServerConfig,
    {"db": DatabaseConfig},
    {"model": {"data_source": "${db.table_cfg}"}},
)  # raises "omegaconf.errors.InterpolationKeyError: Interpolation key 'db.table_config' not found"
$ python omegaconf_failing.py
Traceback (most recent call last):
...
omegaconf.errors.InterpolationKeyError: Interpolation key 'db.table_cfg' not found
    full_key:
    object_type=ServerConfig
Mode 2: merging an interpolation into a structured value
# omegaconf_failing_also.py
from dataclasses import dataclass
from omegaconf import OmegaConf

@dataclass
class TableConfig:
    rows: int = 1

@dataclass
class DatabaseConfig:
    table_cfg: TableConfig = TableConfig()

@dataclass
class ModelConfig:
    data_source: TableConfig = TableConfig()

@dataclass
class ServerConfig:
    db: DatabaseConfig
    model: ModelConfig

cfg = OmegaConf.merge(
    ServerConfig,
    {"db": DatabaseConfig},
    {"model": {"data_source": "${db.table_cfg}"}},
)  # raises "omegaconf.errors.ValidationError: Merge error: str is not a subclass of TableConfig. value: ${db.table_config}"
$ python omegaconf_failing_also.py
Traceback (most recent call last):
...
omegaconf.errors.ValidationError: Merge error: str is not a subclass of TableConfig. value: ${db.table_cfg}
    full_key:
    object_type=ServerConfig

Expected behavior I don't think either merge above should result in an error.

Additional context OmegaConf version: HEAD (commit 8d110fc)

johnzielke commented 1 year ago

Are there any updates on this? If you stumble into this problem, finding out that this is (most likely :D) not a issue in your config but this bug is quite hard to find out.

Jasha10 commented 1 year ago

@johnzielke no updates at this time.

HuangChiEn commented 11 months ago

It may be the stupid answer, but why not

# ...omit!!

cfg = OmegaConf.merge(
    ServerConfig,
    {"db": DatabaseConfig},   # why not just directly set it with dataclass ?
    {"model": {"data_source": TableConfig}},
) 

Suppose, you already have initialized some value

# ...omit!!
# directly assign more clearly then intepolation..
db_tab = DatabaseConfig.table_cfg

cfg = OmegaConf.merge(
    ServerConfig,
    {"db": DatabaseConfig},   # why not just directly set it with db_tab instance ?
    {"model": {"data_source": db_tab}},
) 

The point here: once the instance is initialized, it will be impossible (and not necessary) to do argument interpolation. If you want to sync some arguments between the dataclass configs, just add some code to assign the shallow copy of the value.

Hope this helpful & close the issue ~

jotsif commented 5 months ago

Also having the same problem - would love to have it fixed.