Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create VariadicGreedy input type and remove is_greedy argument from component decorator #7871

Open
silvanocerza opened this issue Jun 14, 2024 · 1 comment
Labels
2.x Related to Haystack v2.0 P2 Medium priority, add to the next sprint if no P1 available topic:core

Comments

@silvanocerza
Copy link
Contributor

silvanocerza commented Jun 14, 2024

When creating a new Component I must decorate it with a @component decorator.
As of now that decorator takes a single bool argument is_greedy as defined here.

That is_greedy argument in turn sets temporarily the __haystack_is_greedy__ internal class dunder field here. The comment says that is done temporarily but, as we see in the following paragraph, is actually wrong. When we started implementing that we assumed it would be temporary because "greediness" of a Component also depends on the type of inputs the it takes, and at that point of the execution we can't yet know reliably know the input types of the Component.

Then here, in the ComponentMeta metaclass, where we can actually reliably check the actual real type of inputs the Component takes we emit a warning letting the user know if they set is_greedy to True and there's no input with Variadic type.

This is necessary because Components that have Variadic input and have is_greedy as True behave in a different way from Components that have Variadic inpot and have is_greedy as False when calling Pipeline.run(). To be precise here we check if a Component has Variadic as input type and also has set is_greedy to True.

The core difference between a Component that have Variadic input and have is_greedy as True and one that has is_greedy as False is that Pipeline.run() will wait as long as possible before trying to execute the Component that has is_greedy as False. While the Component with is_greedy as True will be added to the list of Components that are ready to run as soon as it receives the first input from any sender Component it's connected with.

As this is error prone and "greediness" of a Component is associated to its input types we should actually remove the is_greedy argument. And instead of relying on @component decorator arguments to set this information we should create a VariadicGreedy type, much like we have a Variadic type.

This will make more evident which Component actually is "greedy" and clearly show that "greediness" is associated with Variadic input types.

@silvanocerza
Copy link
Contributor Author

This is also related to #7873 as it talks about execution of Components with Variadic and is_greedy set to True.

"Greediness" of a Component is also referenced in other places during Pipeline.run() execution.

Here we check for the is_greedy field, this could actually be a bug as the is_greedy field is never set and instead __haystack_is_greedy__ is used.

__haystack_is_greedy__ is instead also used here, here and here.

@shadeMe shadeMe added P2 Medium priority, add to the next sprint if no P1 available 2.x Related to Haystack v2.0 topic:core labels Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to Haystack v2.0 P2 Medium priority, add to the next sprint if no P1 available topic:core
Projects
None yet
Development

No branches or pull requests

2 participants