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

[Planning Pipeline] Make planning pipeline respect the max. planning time #2581

Open
sjahr opened this issue Dec 6, 2023 · 4 comments · May be fixed by #2692
Open

[Planning Pipeline] Make planning pipeline respect the max. planning time #2581

sjahr opened this issue Dec 6, 2023 · 4 comments · May be fixed by #2692
Assignees
Labels
enhancement New feature or request persistent Allows issues to remain open without automatic stalling and closing.

Comments

@sjahr
Copy link
Contributor

sjahr commented Dec 6, 2023

Planning time is currently checked in the planner plugin under the assumption that this is the only process time in the pipeline that consumes a meaningful amount of time. This assumption was wrong and got even more invalidated with the recent refactoring that enables chaining planners.

Planning time should be measured and tracked at the pipeline level when the planning algorithms are called

@sjahr sjahr added the enhancement New feature or request label Dec 6, 2023
Copy link

This issue is being labeled as stale because it has been open 45 days with no activity. It will be automatically closed after another 45 days without follow-ups.

@github-actions github-actions bot added the stale Inactive issues and PRs are marked as stale and may be closed automatically. label Jan 22, 2024
@sjahr sjahr added persistent Allows issues to remain open without automatic stalling and closing. and removed stale Inactive issues and PRs are marked as stale and may be closed automatically. labels Jan 22, 2024
@TomCC7
Copy link

TomCC7 commented Feb 11, 2024

Hi @sjahr, I'm interested in working on this issue and have several questions regarding the requirements:

  1. Should we take the request and response adapter processing time into account?
  2. For chained planners, how should we distribute the allowed_planning_time among all planners? It seems to be not optimal if we just let the previous planner use up all the available time. Maybe the planners can share the available time evenly?

For implementation details, it seems like we can directly modify the allowed_planning_time before passing it into the planner here so that we don't need to add extra parameters to the planning request. Does this solution sound viable to you?

@sjahr
Copy link
Contributor Author

sjahr commented Feb 13, 2024

@TomCC7 Thanks for your interest! Feel free to open a WIP PR with your proposed changes and we can discuss based on the code. Regarding your questions:

Should we take the request and response adapter processing time into account?

Yes! That is one of the biggest shortcomings of the current implementations. Planning time should be the sum of each pipeline component's processing time.

For chained planners, how should we distribute the allowed_planning_time among all planners? It seems to be not optimal if we just let the previous planner use up all the available time. Maybe the planners can share the available time evenly?

That's a good question. For a first implementation I would propose a naive time distribution: e.g. evenly distributing the time between the planners. The planners usually require significantly more time than the adapters with the current implementations that's why I think we can give each adapter the full remaining budget and it is very unlikely that a large fraction of it is used.

For implementation details, it seems like we can directly modify the allowed_planning_time before passing it into the planner here so that we don't need to add extra parameters to the planning request.

Yes that's a good idea. Make sure that we manage the remaining budget on the pipeline level. So if planner A is quicker than the max. allowed planning time, the "gained" time can be distributed to the other planners. The exact approach to distribute the time can be discussed in the PR itself. I am pretty sure other maintainers have ideas there too

@TomCC7
Copy link

TomCC7 commented Feb 13, 2024

Thanks for replying! I'll work on this then

@TomCC7 TomCC7 linked a pull request Feb 18, 2024 that will close this issue
3 tasks
@sjahr sjahr linked a pull request Feb 19, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request persistent Allows issues to remain open without automatic stalling and closing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants