refactor: deactivate caching if a component is part of a cycle #3694
+335
−165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
dosubot
bot
added
size:L
ogabrielluiz
force-pushed
the
feat/disablecacheincycle
branch
from
28f3f74
to
286a7c0
Compare
The `has_cycle_edges` method is added to the `Vertex` class to check if the vertex has any cycle edges. Additionally, the `instantiate_component` method is updated to use the `initialize.loading.instantiate_class` function for custom component instantiation.
- Implement `find_cycle_vertices` function to identify all vertices that are part of cycles in a directed graph. - Utilize depth-first search (DFS) to detect cycles and collect vertices involved in those cycles.
- Introduced `_set_cache_to_vertices_in_cycle` method to enable caching for vertices involved in cycles. - Added `find_cycle_vertices` import to support the new method. - Refactored vertex instantiation into `_instantiate_components_in_vertices` method for better code organization.
Refactor the `_set_cache_to_vertices_in_cycle` method to improve caching logic for vertices involved in cycles. Instead of setting the `cache` attribute to `True`, it is now set to `False` for better clarity and consistency. This change ensures that the cache is properly handled for vertices in cycles.
…and add new test case - Removed the `entry_point` parameter from all test cases for `find_cycle_vertices`. - Added a new parameterized test case `test_handle_two_inputs_in_cycle` to verify handling of cycles with two inputs.
…puts in `base.py`
…ntegration - Introduce `test_updated_graph_with_prompts` to validate graph cyclicity and execution. - Integrate `PromptComponent`, `OpenAIModelComponent`, and `ConditionalRouterComponent` in the test. - Ensure graph execution with a maximum of 20 iterations and cache disabled. - Validate the presence of expected output vertices in the results.
…che in cycle vertices
- Introduced `default_value` to handle cases where edges are cycles and target parameters are present. - Ensured that `default_value` is returned if defined, preventing errors when the component is not built.
…date assertions - Simplified component initialization using method chaining. - Corrected router input and message parameters to use openai_component_1. - Updated assertions to check for correct output IDs.
ogabrielluiz
force-pushed
the
feat/disablecacheincycle
branch
from
5b41b5c
to
c091958
Compare
- Introduced `_cycle_vertices` attribute to store vertices involved in cycles. - Added `cycle_vertices` property to compute and cache cycle vertices. - Updated edge creation logic to use `cycle_vertices` for cycle detection.
- Changed router operator from "equals" to "contains". - Consolidated chat output to a single component. - Updated graph construction to use a single chat output. - Replaced `_snapshot` with `get_snapshot` for graph state capture. - Adjusted assertions to reflect the updated graph structure and outputs.
dosubot
bot
added
size:XL
ogabrielluiz
commented
Sep 24, 2024
| @@ -1632,7 +1657,7 @@ def build_edge(self, edge: EdgeData) -> CycleEdge | Edge: | |||
| raise ValueError(f"Source vertex {edge['source']} not found") | |||
| if target is None: | |||
| raise ValueError(f"Target vertex {edge['target']} not found") | |||
| if (source.id, target.id) in self.cycles: | |||
| if any(v in self.cycle_vertices for v in [source.id, target.id]): | |||
There was a problem hiding this comment.
@phact this is what made things start working.
ogabrielluiz
commented
Sep 24, 2024
|
|
||
|
|
||
| @pytest.mark.api_key_required | ||
| def test_updated_graph_with_prompts(): |
There was a problem hiding this comment.
@phact Check out this test to understand a bit more what is happening.
phact
reviewed
Sep 24, 2024
| chat_output = ChatOutput(_id="chat_output") | ||
| chat_output.set(input_value=text_output.text_response) | ||
|
|
||
| graph = Graph(chat_input, chat_output) |
There was a problem hiding this comment.
Not specific to this PR but how come you need to pass an end component to Graph? Wouldn't it eventually get added through the adjacency lists?
There was a problem hiding this comment.
Yeah. This is kinda weird still. We went for a syntax similar to Keras' Functional API.
The end shouldn't be necessary but it has its uses still, I just didn't mind going over it yet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds cycle detection and caching functionality to the
Vertexclass. It introduces thehas_cycle_edgesmethod to check if a vertex has any cycle edges. Additionally, it includes theapply_on_outputsmethod to apply functions to the outputs of a vertex. The pull request also adds a utility function,find_cycle_vertices, to find all vertices that are part of cycles in a directed graph. Theinstantiate_componentmethod is updated to use theinitialize.loading.instantiate_classfunction for custom component instantiation. Unit tests are added for thefind_cycle_verticesutility function. Theset_cache_to_vertices_in_cyclemethod is introduced to enable caching for vertices involved in cycles. The vertex instantiation process is refactored for better code organization.