AutoGPT icon indicating copy to clipboard operation
AutoGPT copied to clipboard

Split `forge.models.config` into `forge.models.config` and `forge.models.state`

Open Pwuts opened this issue 11 months ago • 7 comments

  • Actionable for #6970

    Move clear-cut library code from AutoGPT to Forge (or to /dev/null if Forge already has a better version)

    • ...
    • autogpt.core.configuration
    • ...
  • ⛔ Blocks [all other "Port ... to Forge" items]

    • #7001
  • Separation Currently, autogpt.core.configuration handles both configuration and state management. It makes sense to separate those functions into forge.config and forge.state. For example:

    # BEFORE
    class MyComponent(Configurable[MySettings]):
        default_settings = MySettings(
            configuration=MyConfig(
                enable_coolness=True
            ),
            some_stateful_stuff=MyPartialState(
                coolness_counter=0,
                max_coolness=math.inf,
            ),
        )
    
    # AFTER
    C = TypeVar("C", bound=SystemConfiguration)
    class Configurable(Generic[C]):
        config_type: ClassVar[type[C]]
        config: C
    
        @classmethod
        def _make_config(cls, **overrides) -> C:
            """
            Builds the configuration from the following sources (in order of precedence):
            1. Overrides
            2. Environment variables
            3. Default values on the config_type
            """
            # insert implementation here
    
        def __init__(self, **kwargs):
            self.config = self._make_config(**kwargs)
            super(Configurable, self).__init__(**kwargs)
    
    S = TypeVar("S", bound=SystemState)
    class Stateful(Generic[S]):
        _state_type: ClassVar[type[S]]
        _state: S
    
        def __init__(self, state: Optional[S] = None, **kwargs):
            self._state = state or self._state_type()
            super(Stateful, self).__init__(**kwargs)
    
        def export_state(self) -> S:
            return self._state
    
    class MyConfig(SystemConfiguration):
        enable_coolness: bool = UserConfigurable(default=True)
        max_coolness: int = math.inf
    
    class MyState(SystemState):
        coolness_counter: int = 0
    
    class Component(Configurable[MyConfig], Stateful[MyState]):
        def __init__(self):
            super(Component, self).__init__()
    
        def some_method(self):
            if self.config.enable_coolness:
                if self._state.coolness_counter + 1 > self.config.max_coolness:
                    raise ValueError("too cool!")
                self._state.coolness_counter += 1
    
    class AltComponent(Configurable[MyConfig], Stateful[MyState]):
        def __init__(self):
            super(AltComponent, self).__init__()
    
            self.counter = self._state.coolness_counter
    
        def some_method(self):
            if self.config.enable_coolness:
                if self.counter + 1 > self.config.max_coolness:
                    raise ValueError("too cool!")
                self.counter += 1
    
        def export_state(self) -> MyState:
            return MyState(coolness_counter=self.counter)
    

    Configurable+SystemConfiguration and Stateful+SystemState will share a large portion of their implementations, because both support the same kind of nesting and related functionality.

Background

  • #6970
  • Other modules to be ported from AutoGPT to Forge depend on autogpt.core.configuration
  • Most applications need config and state management, so this is useful anyways

Notes

  • In the future it may make sense to move to an externally imported config library. This is purposefully not included in this issue to keep the task manageable.

Pwuts avatar Mar 11 '24 14:03 Pwuts

This issue has automatically been marked as stale because it has not had any activity in the last 50 days. You can unstale it by commenting or removing the label. Otherwise, this issue will be closed in 10 days.

github-actions[bot] avatar May 01 '24 01:05 github-actions[bot]

This issue was closed automatically because it has been stale for 10 days with no activity.

github-actions[bot] avatar May 12 '24 01:05 github-actions[bot]

Unstale

ntindle avatar May 12 '24 08:05 ntindle

This issue has automatically been marked as stale because it has not had any activity in the last 50 days. You can unstale it by commenting or removing the label. Otherwise, this issue will be closed in 10 days.

github-actions[bot] avatar Jul 02 '24 01:07 github-actions[bot]

Partially implemented in #7106

Pwuts avatar Jul 02 '24 01:07 Pwuts

@kcze do you think splitting is useful/necessary? Otherwise we can close this.

Pwuts avatar Jul 02 '24 01:07 Pwuts

@kcze do you think splitting is useful/necessary? Otherwise we can close this.

I think it's useful, config.py is quite large. We could rename *Settings to *State in the same PR as well.

kcze avatar Jul 03 '24 13:07 kcze

This issue has automatically been marked as stale because it has not had any activity in the last 50 days. You can unstale it by commenting or removing the label. Otherwise, this issue will be closed in 10 days.

github-actions[bot] avatar Aug 23 '24 01:08 github-actions[bot]