Add config classes #591

collindutter · 2024-01-18T00:48:20Z

Closes #242, #509

This PR introduces the following changes:

Added

OpenAiStructureConfig for providing Structures with all OpenAi Driver configuration.
AmazonBedrockStructureConfig for providing Structures with all Amazon Bedrock Driver configuration.
StructureConfig for building your own Structure configuration.
JsonExtractionTask for convenience over using ExtractionTask with a JsonExtractionEngine.
CsvExtractionTask for convenience over using ExtractionTask with a CsvExtractionEngine.

Changed

Deprecated Structure.prompt_driver in favor of Structure.global_drivers.prompt_driver.
Deprecated Structure.embedding_driver in favor of Structure.global_drivers.embedding_driver.
Deprecated Structure.stream in favor of Structure.global_drivers.prompt_driver.stream.
TextSummaryTask.summary_engine now defaults to a PromptSummaryEngine with a Prompt Driver default of Structure.global_drivers.prompt_driver.
TextQueryTask.query_engine now defaults to a VectorQueryEngine with a Prompt Driver default of Structure.global_drivers.prompt_driver and Vector Store Driver default of Structure.global_drivers.vector_store_driver.
PromptImageGenerationTask.image_generation_engine now defaults to a PromptImageGenerationEngine with an Image Generation Driver default of Structure.global_drivers.image_generation_driver.
VariationImageGenerationTask.image_generation_engine now defaults to a VariationImageGenerationEngine with an Image Generation Driver default of Structure.global_drivers.image_generation_driver.
InpaintingImageGenerationTask.image_generation_engine now defaults to an InpaintingImageGenerationEngine with an Image Generation Driver default of Structure.global_drivers.image_generation_driver.
OutpaintingImageGenerationTask.image_generation_engine now defaults to an OutpaintingImageGenerationEngine with an Image Generation Driver default of Structure.global_drivers.image_generation_driver.

Some usage patterns that come from this:

Configuring a Structure to use Bedrock:

agent = Agent(
    config=AmazonBedrockStructureConfig()
)

Configuring a Structure to use Anthropic:

agent = Agent(
    config=StructureConfig(
        global_drivers=StructureGlobalDriversConfig(
            prompt_driver=AnthropicPromptDriver(
                model="claude-2",
                api_key=os.environ["ANTHROPIC_API_KEY"],
            )
        )
    ),
)

Overriding a Structure's Task Memory Query Engine's Embedding Driver:

agent = Agent(
    config=StructureConfig(
        task_memory=StructureTaskMemoryConfig(
            query_engine=StructureTaskMemoryQueryEngineConfig(
                vector_store_driver=LocalVectorStoreDriver(
                    embedding_driver=OpenAiEmbeddingDriver(),
                )
            )
        )
    )
)

Modifying an existing config + serializing/deserializing:

custom_config = AmazonBedrockStructureConfig()
custom_config.global_drivers.embedding_driver = AmazonBedrockCohereEmbeddingDriver()
custom_config.merge_config(
    {
        "task_memory": {
            "summary_engine": {
                "prompt_driver": {
                    "model": "amazon.titan-text-express-v1",
                    "prompt_model_driver": {
                        "type": "BedrockTitanPromptModelDriver",
                    },
                }
            }
        }
    }
)
serialized_config = custom_config.to_json()
deserialized_config = AmazonBedrockStructureConfig.from_json(serialized_config)

agent = Agent(
    config=deserialized_config,
)

andrewfrench · 2024-02-07T17:01:17Z

@collindutter #635 updated the TextQueryTask to depend on VectorQueryEngine rather than BaseQueryEngine but only added top_n to VectorQueryEngine. Your change here still references BaseQueryEngine, as it probably should, so I added top_n to the base class in #640

andrewfrench · 2024-02-07T23:36:17Z

Code changes look great and after playing with the features this is a clear huge step forward. One question @collindutter is that while Tasks look up to Structures to inherit configurations (like Engines), Tools don't do this. Tools also don't have an existing reference to a Structure that they can look to, but is this intentional? It seems like a natural convenience (even if in a later PR) that an Agent, for example, is provided with a configuration and the developer doesn't have to specifically re-wire its Tools.

collindutter · 2024-02-07T23:40:06Z

Thanks for the feedback and for trying it out! Yes this is something that Tools should also have since they're basically a sibling abstraction to Tasks. I only extended the functionality here because it was already partially implemented in Prompt Task with a look up to Prompt Driver. Agreed that in a future PR we should evaluate how we can better share logic between Tasks and Tools.

andrewfrench

Big fan, can't wait to type less.

griptape/memory/structure/summary_conversation_memory.py

vasinov

Awesome work! Should make it way easier to use and extend the framework!

vasinov · 2024-02-09T20:42:34Z

griptape/config/amazon_bedrock_structure_config.py

+
+@define(kw_only=True)
+class AmazonBedrockStructureConfig(BaseStructureConfig):
+    global_drivers: StructureGlobalDriversConfig = field(


Should we rename it to global and StructureGlobalConfig in case we add more things, other than drivers, to it in the future?

Or should we keep it namespaced to drivers for clarity and just introduce another config "bundle" in the future?

I'd prefer to keep it namespaced; I could see a singular global becoming quite unruly.

vasinov · 2024-02-09T20:46:47Z

griptape/config/openai_structure_config.py

+)
+
+
+@define(kw_only=True)


Should we apply this to all new classes in this PR and, in a separate PR, to most (or all?) classes in Griptape?

I think I'd actually vote to remove it from this PR, and do it in a single sweep in another.

Yeah, let's remove it.

vasinov · 2024-02-09T20:50:20Z

griptape/config/structure_global_drivers_config.py

+@define(kw_only=True)
+class StructureGlobalDriversConfig(SerializableMixin):
+    prompt_driver: BasePromptDriver = field(
+        kw_only=True, default=Factory(lambda: NopPromptDriver()), metadata={"serializable": True}


Just wanted to make sure this is the best convention...do we like NoOp, Null, Dummy, Stub, or Empty better? Or should we stick with Nop?

I like Stub. Nop and NoOp feel a bit technical, and Null feels dangerous.

What do you think @andrewfrench?

Traditionally I use Dummy

Good with me too. Before I do a big refactor, @vasinov are you also good with Dummy? DummyPromptDriver?

That works! It's more fun than Stub and Stub is too Java-y anyways :)

vasinov · 2024-02-09T20:53:14Z

griptape/config/structure_task_memory_extraction_engine_config.py

+
+
+@define(kw_only=True)
+class StructureTaskMemoryExtractionEngineConfig(SerializableMixin):


Random thought: will we ever want to use configs as mixins? In other words, will it ever make sense to inherit from two configs at once? If that's the case, should we add the Mixin suffix to config files and @define(slots=False)?

I'm having a difficult time imaging a configuration situation where we'd inherit from multiple. Did you have something specific in mind?

Nothing specific, just wondering. Let's keep it the way it is then :)

vasinov · 2024-02-09T21:18:28Z

griptape/drivers/image_generation/nop_image_generation_driver.py

+class NopImageGenerationDriver(BaseImageGenerationDriver):
+    model: str = field(init=False)
+
+    def try_text_to_image(self, prompts: list[str], negative_prompts: Optional[list[str]] = None) -> ImageArtifact:


I know it was established in another PR but why do we prefix those driver methods with try_?

Because they have parent methods that wrap them in extra logic. Just following the pattern of other Drivers like try_run in Prompt Driver.

Got it—makes sense!

vasinov · 2024-02-09T21:19:02Z

griptape/drivers/image_generation/openai_image_generation_driver.py

-        Literal["256x256"] | Literal["512x512"] | Literal["1024x1024"] | Literal["1024x1792"] | Literal["1792x1024"]
-    ) = field(default="1024x1024", kw_only=True)
-    response_format: Literal["b64_json"] = field(default="b64_json", kw_only=True)
+        Union[Literal["256x256"], Literal["512x512"], Literal["1024x1024"], Literal["1024x1792"], Literal["1792x1024"]]


Are we back to Union over | everywhere in the framework?

Need to use Union for class fields as long as we're supporting python 3.9. We had a slack thread on this somewhere I can try to dig up.

Ah, yes, I remember now.

vasinov · 2024-02-09T21:21:36Z

griptape/drivers/prompt/base_prompt_driver.py

+                        if self.structure:
+                            self.structure.publish_event(CompletionChunkEvent(token=chunk.value))
+                        else:
+                            raise ValueError("Streaming requires a structure")


This feels a bit off: shouldn't prompt drivers still be able to stream regardless of the presence of the structure?

I agree, but in the current implementation, publishing events requires a Structure to be set.

I can revert this change since it's kind of out of scope of an already massive PR, but I think we need a separate issue to decouple Prompt Drivers and Structures. Maybe Prompt Drivers take an Event Listener instead.

EDIT: separate issue

Sounds good. Let's address it in the separate issue.

vasinov · 2024-02-09T22:37:22Z

griptape/structures/structure.py

+    def default_config(self) -> BaseStructureConfig:
+        config = OpenAiStructureConfig()
+
+        if self.prompt_driver is not None:


We should use a more pythonic @deprecated decorator for those things:

@property @deprecated("use `config.global_drivers.prompt_driver` instead.") def prompt_driver(self) -> Optional[BasePromptDriver]: return None

Added a deprecated decorator and refactored.

vasinov · 2024-02-09T22:39:00Z

griptape/structures/structure.py

+                            if not isinstance(task_memory.query_engine.prompt_driver, NopPromptDriver)
+                            else global_drivers.prompt_driver


Can we flip it here and below to not use a negative condition, so it's easier to read and reason about?

vasinov · 2024-02-09T22:39:34Z

griptape/tasks/base_text_input_task.py

@@ -15,7 +15,9 @@
 class BaseTextInputTask(RuleMixin, BaseTask, ABC):
    DEFAULT_INPUT_TEMPLATE = "{{ args[0] }}"

-    _input: str | TextArtifact | Callable[[BaseTask], TextArtifact] = field(default=DEFAULT_INPUT_TEMPLATE)
+    _input: str | TextArtifact | Callable[[BaseTask], TextArtifact] = field(


No Union here? :)

The use of Union is really only required if the field is marked as serializable. Related to this comment.

collindutter added 30 commits January 3, 2024 09:51

Add config classes

2960468

Update config layout

e0fbd0e

Create marshmallow schemas

5f90b95

Dynamic serialization mixin untested

88d12a3

Some progress with cattrs

5ff1db0

Partially working cattrs unstructure

32ce57c

Partially working marshmallow attrs

d9234c3

Dynamic schema loading and dumping works

eecd9fc

Merge branch 'dev' into feature/config

0ab1e21

Delete config schemas

a072af9

Remove extra space

41d998e

Remove drivers schemas

c0579aa

Fix tests

921bdc8

Fix loading

7e75e06

Merge branch 'dev' into feature/config

9ebad74

Auto serialize artifacts

a9a91b3

Auto serialize events

c1504e5

Auto serialize memory

7b1ec6d

Misc fixes

942ca51

In progress of deleting all schemas

71046ce

Merge branch 'dev' into refactor/auto-serialize

c58b340

In progress refactor

5504e93

Fix field name

2ac97b0

Mostly working

140be19

Remove config

11baf50

Check ABC for polymorphic

2c944c3

Fix tests

35004b3

Remove old libs

36da3b7

Revert type remove

9d4832e

Cleanup

5b78e8b

collindutter added 10 commits February 7, 2024 09:28

Merge branch 'dev' into feature/default-config

c71ddb1

Change how tasks use engines

815e3ea

Fix check

54ebb24

Fix tests

4e84d01

Add nop tests

137a981

More nop changes

9a05f8e

Add tests for task properties

bc92356

Mock aws creds

4ae312c

Mock region

81172e1

Update changelog

d2b66fa

collindutter force-pushed the feature/default-config branch from 6e5639d to d2b66fa Compare February 7, 2024 17:28

collindutter mentioned this pull request Feb 7, 2024

OpenAI Vision image query functionality #625

Merged

andrewfrench previously approved these changes Feb 8, 2024

View reviewed changes

andrewfrench reviewed Feb 8, 2024

View reviewed changes

griptape/memory/structure/summary_conversation_memory.py Show resolved Hide resolved

vasinov requested changes Feb 9, 2024

View reviewed changes

collindutter added 2 commits February 12, 2024 09:01

Remove kw_only from @define

503bc47

Revert structure check

1b668b0

collindutter dismissed andrewfrench’s stale review via 1b668b0 February 12, 2024 17:21

collindutter added 2 commits February 12, 2024 09:24

Negate conditions to improve readability

6342b1a

Refactor deprecated warning

5c01d68

collindutter force-pushed the feature/default-config branch from 6a69dca to 5c01d68 Compare February 12, 2024 17:58

collindutter requested a review from vasinov February 12, 2024 18:00

Rename Nop to Dummy

da8b50d

collindutter requested a review from andrewfrench February 12, 2024 19:59

vasinov approved these changes Feb 12, 2024

View reviewed changes

collindutter merged commit c87c85f into dev Feb 12, 2024
6 checks passed

collindutter deleted the feature/default-config branch February 12, 2024 20:43



		@define(kw_only=True)
		class StructureTaskMemoryExtractionEngineConfig(SerializableMixin):

		if not isinstance(task_memory.query_engine.prompt_driver, NopPromptDriver)
		else global_drivers.prompt_driver

Add config classes #591

Add config classes #591

Conversation

collindutter commented Jan 18, 2024 • edited Loading

Added

Changed

andrewfrench commented Feb 7, 2024

andrewfrench commented Feb 7, 2024

collindutter commented Feb 7, 2024

andrewfrench left a comment

Choose a reason for hiding this comment

vasinov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collindutter Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collindutter Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collindutter commented Jan 18, 2024 •

edited

Loading

collindutter Feb 9, 2024 •

edited

Loading

collindutter Feb 12, 2024 •

edited

Loading