Nested config values are not updated with provided value in --config file

[gbmarc1]

Describe the bug
The value provided in a config file is not propagated to the nested instances of the config. However, when provided directly via command line, it does work.

To Reproduce
This is a simplified config and code:

from spock.builder import ConfigArgBuilder
from spock.config import spock

@spock
class TrainProcess:
    epochs: int = 100

@spock
class Train:
    train_process: TrainProcess = TrainProcess()



def main():
    attrs_class = ConfigArgBuilder(
        Train, TrainProcess,
        desc='',
    ).generate()
    print(attrs_class)


if __name__ == '__main__':
    main()

The provided command is:
train --config best_params.yaml

with best_params.yaml

epochs: 1

Expected behavior
--config values should be propagated to nested config instances like values from command line

Screenshots
Snipping of debug pane for attrs_class object

Additional context
spock-config==2.2.6

[gbmarc1]

For people like me that did not understood the Nested stuff. You need to have the following in the config files:

TrainProcess:
    epochs: 1

[ncilfone]

Apologies for the slow follow up as I was away for the weekend. Yeah this is buried in the docs here and not super visible. Another pass through the docs for clarity is on my radar.

[gbmarc1]

Hi, sorry so by testing a bit more again, it seems it does not work with 2 levels of nested config.

from spock.builder import ConfigArgBuilder
from spock.config import spock

@spock
class AnotherNested:
    something: int = 5

@spock
class TrainProcess:
    epochs: int = 100
    nest: AnotherNested = AnotherNested()

@spock
class Train:
    train_process: TrainProcess = TrainProcess()



def main():
    attrs_class = ConfigArgBuilder(
        Train, TrainProcess, AnotherNested
        desc='',
    ).generate()
    print(attrs_class)


if __name__ == '__main__':
    main()

The --config

AnotherNested:
    something: 1

The config arg builder will not update the nested AnotherNested config within Train->TrainProcess. But the config at the TOP level will be updated a per the --config value

[ncilfone]

@gbmarc1 Looking at this now... seems like a mapping issue within the spock backend since the child object seems to get updated with the --config defined value but not the parent

[gbmarc1]

Hi thank you for looking at this.

I can't seem to find the recursivity in the code where config are instantiated with the --config values

[ncilfone]

Hi @gbmarc1. I think #160 should fix this. Weird edge case that was happening that wouldn't call the recursive checking. Thanks for finding!

I'll pull it into master if you want to check and see if it works on your end, and then I can cut a minor release bump so there is a pip wheel...

[gbmarc1]

I still have the problem after installing
pip install git+https://github.com/fidelity/spock.git@master

[ncilfone]

Running the following on @master

# -*- coding: utf-8 -*-
from spock.builder import ConfigArgBuilder
from spock.config import spock


@spock
class AnotherNested:
    something: int = 5


@spock
class TrainProcess:
    epochs: int = 100
    nest: AnotherNested = AnotherNested()


@spock
class Train:
    train_process: TrainProcess = TrainProcess()


def main():
    attrs_class = ConfigArgBuilder(
        Train, TrainProcess, AnotherNested,
        desc='',
    ).generate()
    print(attrs_class)


if __name__ == '__main__':
    main()

AnotherNested:
    something: 1

Produces this:

AnotherNested: !!python/object:spock.backend.config.AnotherNested
  something: 1
Train: !!python/object:spock.backend.config.Train
  train_process: !!python/object:spock.backend.config.TrainProcess
    epochs: 100
    nest: !!python/object:spock.backend.config.AnotherNested
      something: 1
TrainProcess: !!python/object:spock.backend.config.TrainProcess
  epochs: 100
  nest: !!python/object:spock.backend.config.AnotherNested
    something: 1

[gbmarc1]

I agree the previous code works. But one of my test fails

[ncilfone]

Do these all come in from default or via the config file?

[gbmarc1]

class MyEnum(Enum):
    value_1 = "value_1"
    value_2 = "value_2"


@spock
class AnotherNestedConfig:
    something: int = 5


@spock
class NestedConfig:
    integer: int = 1
    my_enum: MyEnum = "value_1"
    other_nest: AnotherNestedConfig = AnotherNestedConfig()


@spock
class DuplicatedConfig:
    flag_3: bool = False


@spock
class MakeDatasetConfig:
    flag1: bool = False
    flag_2: bool = False
    nested_config: NestedConfig = NestedConfig()
    duplicated_config: DuplicatedConfig = DuplicatedConfig()


@spock
class BuildFeatureConfig:
    duplicated_config: DuplicatedConfig = DuplicatedConfig()

# Write spock-config config file
with open("config.yaml", "w") as f:
    yaml.dump(
        {"NestedConfig": {"integer": 2}, "AnotherNestedConfig": {"something": 1}},
        f,
    )
 sys.argv = [".py", "--config", "config.yaml"]

[ncilfone]

Running this:

# -*- coding: utf-8 -*-
from spock.builder import ConfigArgBuilder
from spock.config import spock
from enum import Enum


class MyEnum(Enum):
    value_1 = "value_1"
    value_2 = "value_2"


@spock
class AnotherNestedConfig:
    something: int = 5


@spock
class NestedConfig:
    integer: int = 1
    my_enum: MyEnum = "value_1"
    other_nest: AnotherNestedConfig = AnotherNestedConfig()


@spock
class DuplicatedConfig:
    flag_3: bool = False


@spock
class MakeDatasetConfig:
    flag1: bool = False
    flag_2: bool = False
    nested_config: NestedConfig = NestedConfig()
    duplicated_config: DuplicatedConfig = DuplicatedConfig()


@spock
class BuildFeatureConfig:
    duplicated_config: DuplicatedConfig = DuplicatedConfig()




# @spock
# class AnotherNested:
#     something: int = 5
#
#
# @spock
# class NestedConfig:
#     integer: int = 2
#     other_nest: AnotherNested = AnotherNested()
#
#
# @spock
# class TrainProcess:
#     epochs: int = 100
#     nest: NestedConfig = NestedConfig()
#
#
# @spock
# class Train:
#     train_process: TrainProcess = TrainProcess()


def main():
    attrs_class = ConfigArgBuilder(
        BuildFeatureConfig, MakeDatasetConfig, DuplicatedConfig, NestedConfig, AnotherNestedConfig,
        desc='',
    ).generate()
    print(attrs_class)
    #
    # attrs_class = ConfigArgBuilder(
    #     Train, TrainProcess, AnotherNested, NestedConfig,
    #     desc='',
    # ).generate()
    # print(attrs_class)


if __name__ == '__main__':
    main()

and the --config yaml:

AnotherNestedConfig:
    something: 1

Leads to this...

AnotherNestedConfig: !!python/object:spock.backend.config.AnotherNestedConfig
  something: 1
BuildFeatureConfig: !!python/object:spock.backend.config.BuildFeatureConfig
  duplicated_config: !!python/object:spock.backend.config.DuplicatedConfig
    flag_3: false
DuplicatedConfig: !!python/object:spock.backend.config.DuplicatedConfig
  flag_3: false
MakeDatasetConfig: !!python/object:spock.backend.config.MakeDatasetConfig
  duplicated_config: !!python/object:spock.backend.config.DuplicatedConfig
    flag_3: false
  flag1: false
  flag_2: false
  nested_config: !!python/object:spock.backend.config.NestedConfig
    integer: 1
    my_enum: value_1
    other_nest: !!python/object:spock.backend.config.AnotherNestedConfig
      something: 1
NestedConfig: !!python/object:spock.backend.config.NestedConfig
  integer: 1
  my_enum: value_1
  other_nest: !!python/object:spock.backend.config.AnotherNestedConfig
    something: 1

Am I missing something?

[gbmarc1]

# -*- coding: utf-8 -*-
from spock.builder import ConfigArgBuilder
from spock.config import spock
from enum import Enum


class MyEnum(Enum):
    value_1 = "value_1"
    value_2 = "value_2"


@spock
class AnotherNestedConfig:
    something: int = 5


@spock
class NestedConfig:
    integer: int = 1
    my_enum: MyEnum = "value_1"
    other_nest: AnotherNestedConfig = AnotherNestedConfig()


@spock
class DuplicatedConfig:
    flag_3: bool = False


@spock
class MakeDatasetConfig:
    flag1: bool = False
    flag_2: bool = False
    nested_config: NestedConfig = NestedConfig()
    duplicated_config: DuplicatedConfig = DuplicatedConfig()


@spock
class BuildFeatureConfig:
    duplicated_config: DuplicatedConfig = DuplicatedConfig()


def main():
    attrs_class = ConfigArgBuilder(
        BuildFeatureConfig, MakeDatasetConfig, DuplicatedConfig, NestedConfig, AnotherNestedConfig,
        desc='',
    ).generate()
    print(attrs_class)

import sys
from ruamel.yaml import YAML
if __name__ == '__main__':
    sys.argv = [".py", "--config", "config.yaml"]
    yaml = YAML(typ="safe")
    with open("config.yaml", "w") as f:
        yaml.dump(
            {"NestedConfig": {"integer": 2}, "AnotherNestedConfig": {"something": 1}},
            f,
        )
    main()

I get this

AnotherNestedConfig: !!python/object:spock.backend.config.AnotherNestedConfig
  something: 1
BuildFeatureConfig: !!python/object:spock.backend.config.BuildFeatureConfig
  duplicated_config: !!python/object:spock.backend.config.DuplicatedConfig
    flag_3: false
DuplicatedConfig: !!python/object:spock.backend.config.DuplicatedConfig
  flag_3: false
MakeDatasetConfig: !!python/object:spock.backend.config.MakeDatasetConfig
  duplicated_config: !!python/object:spock.backend.config.DuplicatedConfig
    flag_3: false
  flag1: false
  flag_2: false
  nested_config: !!python/object:spock.backend.config.NestedConfig
    integer: 2
    my_enum: value_1
    other_nest: !!python/object:spock.backend.config.AnotherNestedConfig
      something: 5
NestedConfig: !!python/object:spock.backend.config.NestedConfig
  integer: 2
  my_enum: value_1
  other_nest: !!python/object:spock.backend.config.AnotherNestedConfig
    something: 1


Process finished with exit code 0

I get a something = 5

[ncilfone]

ahhhh ok got it now... seems like there is still a bug in the recursivity that is overwriting some of the config based values. Investigating now

[ncilfone]

@gbmarc1 I'm going to be out for a few weeks so resolution might not come swift on this :-/

Can you just avoid some of the nesting for now? you can always just pass around multiple Spockspaces albeit a little more verbose... I'll fix it when I'm back in early Dec.

If your interested, I think the solution here is to be more pedantic when dealing with nesting by building a directed graph of parent -> child relations such that the overall config can be built recursively by a DFS pattern... This in theory is what it's doing with the recursive calls it's just that it's not directly factored into a graph first. I think this should solve all these problems since it will be much more traceable

[gbmarc1]

@ncilfone I can try to resolve the problem if you point me in the right direction. Where do you think the problem lies in the code?
I will make a pull request that you will be able to agree at your return.

[ncilfone]

@gbmarc1 I might have a really ugly temp fix that would be a bandaid in the meantime... I'll leave it on a branch for a PR but not rope it into master in case it causes chaos elsewhere....

hang tight...

[ncilfone]

@gbmarc1 Also happy to have you contribute as well! This is where all the 'late defaults' get set... aka classes that come in from config files that might need recursing:

spock/spock/backend/builder.py

Line 230 in 27cbcbb

self._handle_late_defaults(args, fields, input_class)

The _handle_late_defaults function is where there is just a bunch of spaghetti boolean logic to catch all these edge cases.

My thought is that here:

spock/spock/backend/builder.py

Line 184 in 27cbcbb

def generate(self, dict_args):

Before starting to iterate through all the input classes (the spock objects that are defined but not instantiated yet with the default values or values from the config/cmd-line) build a directed graph of all classes. Then you should be just be able to DFS through that and set the correct values from the dict_args value in the child classes and then use those as the values of the dictionary (**kwargs that get passed to the actual instantiation of the object) for the parents working up the graph...

But this is just a half-baked thought to start with... I think that this or something else graph/chil-parent based will be much less spaghetti boolean recursive logic

[ncilfone]

Try this branch that has the ugly patch:

https://github.com/fidelity/spock/tree/nested_fix

[ncilfone]

Actually closed in #181 to give attribution.