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

[Docs] Docstring for map_task's min_success_ratio #4623

Open
2 tasks done
Tracked by #5783
artgillespie opened this issue Dec 19, 2023 · 4 comments
Open
2 tasks done
Tracked by #5783

[Docs] Docstring for map_task's min_success_ratio #4623

artgillespie opened this issue Dec 19, 2023 · 4 comments
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest

Comments

@artgillespie
Copy link

Description

See Slack thread for more context.

I found the docstring

:param min_success_ratio: If specified, this determines the minimum fraction of total jobs which must complete
            successfully before terminating this task and marking it unsuccessful

to be somewhat unclear. Specifically, map_task will map across all inputs as long as min_success_ratio is satisfied and only terminate early when the number of failed tasks exceeds 1.0 - min_success_ratio

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@artgillespie artgillespie added documentation Improvements or additions to documentation untriaged This issues has not yet been looked at by the Maintainers labels Dec 19, 2023
Copy link

welcome bot commented Dec 19, 2023

Thank you for opening your first issue here! 🛠

Copy link

dosubot bot commented Dec 19, 2023

🤖

Hello @artgillespie! I'm Dosu, a friendly bot here to help you while we wait for a real human maintainer. I'm here to assist with bugs, answer questions, and guide you through contributing. Remember, no question is too small or too big for me! Let's work together on this.

Thank you for your feedback on the map_task function's min_success_ratio parameter. I understand that the current docstring might be unclear, and I appreciate your suggestion for improvement.

Based on the context provided, the min_success_ratio parameter is used to determine the minimum ratio of successful sub-tasks required for the overall task to be considered successful. This is particularly useful in scenarios where a task is being executed in parallel across multiple nodes, and it's acceptable for a certain percentage of those nodes to fail without causing the entire task to fail.

In the Flyte codebase, the min_success_ratio parameter is used in the TestHandleArrayNodePhaseExecuting function. Here, it is used to determine whether the array node phase should transition to succeeding or failing based on the success ratio of the subnodes. For example, in the test case named "OneSubNodeSuccedeedMinSuccessRatio", the min_success_ratio is set to 0.5. This means that at least 50% of the subnodes must succeed for the array node to be considered successful. In this test case, one subnode succeeds and one fails, so the success ratio is 0.5, which meets the min_success_ratio. Therefore, the expected array node phase is v1alpha1.ArrayNodePhaseSucceeding.

In contrast, in the test case named "OneSubNodeFailed", there is no min_success_ratio set, so the default behavior is that all subnodes must succeed for the array node to be considered successful. In this test case, one subnode fails, so the expected array node phase is v1alpha1.ArrayNodePhaseFailing.

You can find these test cases in the flytepropeller/pkg/controller/nodes/array/handler_test.go file.

Based on this, a more accurate docstring for the min_success_ratio parameter could be:

:param min_success_ratio: This determines the minimum ratio of successful sub-tasks required for the overall task to be considered successful. If the success ratio of the sub-tasks falls below this value, the task will be terminated and marked as unsuccessful.

I hope this clarifies the purpose and usage of the min_success_ratio parameter. If you have any further questions or suggestions, please feel free to share them.

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@hamersaw hamersaw removed their assignment Dec 22, 2023
@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Dec 22, 2023
@neverett neverett added the backlogged For internal use. Reserved for contributor team workflow. label Jan 11, 2024
@0xSolanaceae
Copy link

Is this something I can work on?

@davidmirror-ops
Copy link
Contributor

@0xSolanaceae sure! let us know soon if you have questions. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. documentation Improvements or additions to documentation good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

No branches or pull requests

6 participants