-
Notifications
You must be signed in to change notification settings - Fork 43
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
UP-BFGP interface v0.2.0, few-shot engine and BFGP++ generalized planner v0.1.0 #514
base: master
Are you sure you want to change the base?
Conversation
Gp planner
Aiplan4eu master
Final Deliverable merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @jsego! (And sorry for the latency)
This is super-interesting, I left some comments and suggestions, let me know what you think.
def is_fewshot_planner() -> bool: | ||
return True | ||
|
||
def solve( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename this method, otherwise it would conflict with the OneshotPlanner
Operation Mode, making it impossible to have an engine supporting both FewshotPlanner
and OneshotPlanner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a OneshotPlanner a specialization of a FewshotPlanner, so if some software could do both, then it is a FewshotPlanner, i.e. a OneshotPlanner is a FewshotPlanner that always works with one instance.
This method takes a list of `AbstractProblem`s and returns a `PlanGenerationResult`, | ||
which contains information about the solution to the problems given by the generalized planner. | ||
|
||
:param problems: is the list of `AbstractProblem`s to solve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any assumption on the structure of these problems? What happens if they come from radically different domains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have such assumptions we need to make them explicit in the OM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It assumes the domain is shared for all instances.
You can see an example in the UP-BFGP interface.
) -> List["up.engines.results.PlanGenerationResult"]: | ||
""" | ||
This method takes a list of `AbstractProblem`s and returns a `PlanGenerationResult`, | ||
which contains information about the solution to the problems given by the generalized planner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain how this is different from calling a OneshotPlanner
n-times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main purpose of a FewshotPlanner
is to produce domain-specific knowledge that i) inform about the structure and complexity of the solution for any instance in that domain, and ii) scales up to larger and more complex planning instances. Thus, it must be more scalable than calling a classical planner n
times on new and larger planning instances, if the domain knowledge is computable in reasonable time and memory.
return self._solve(problems, heuristic, timeout, output_stream) | ||
|
||
@abstractmethod | ||
def _solve( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this method neds to be renamed to avoid clash with OneshotPlanner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Commented above)
This code is required for the Final Deliverable of the "Integration of a Generalized Planner in the Unified Planning Framework" project.
The PR includes a new engine to do few-shot planning, where the input is a set of problems in a given domain. Also registers a new planner named BFGP whose interface can be found in the up-bfgp repository.