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

Feat: Create STN from a TTP #509

Merged
merged 41 commits into from
Jan 15, 2024
Merged

Feat: Create STN from a TTP #509

merged 41 commits into from
Jan 15, 2024

Conversation

anissaab
Copy link
Collaborator

No description provided.

@anissaab anissaab self-assigned this Oct 27, 2023
Copy link
Contributor

@Framba-Luca Framba-Luca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work. 😃

It still needs some interface work before merging, but I discovered I will probably have to do it so I wrote mostly general tips.

unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
unified_planning/plans/ttp_to_stn.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (b44c7bc) 84.91% compared to head (1306b46) 85.18%.
Report is 21 commits behind head on master.

❗ Current head 1306b46 differs from pull request most recent head 34e1599. Consider uploading reports for the commit 34e1599 to get more accurate results

Files Patch % Lines
unified_planning/plans/time_triggered_plan.py 94.07% 8 Missing ⚠️
unified_planning/test/examples/realistic.py 97.70% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #509      +/-   ##
==========================================
+ Coverage   84.91%   85.18%   +0.26%     
==========================================
  Files         200      201       +1     
  Lines       26422    26746     +324     
==========================================
+ Hits        22436    22783     +347     
+ Misses       3986     3963      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nicola-gigante
Copy link

Hello! This would be very valuable for me in a project I'm starting. Will this work on TimeTriggeredPlans obtained from a HierarchicalProblem with durative actions?

@anissaab
Copy link
Collaborator Author

anissaab commented Dec 5, 2023

Hello! This would be very valuable for me in a project I'm starting. Will this work on TimeTriggeredPlans obtained from a HierarchicalProblem with durative actions?

Hi ! It shouldn't be a problem ! If you use a TimeTriggeredPlan and a problem with durative actions you should obtains a STN. The Problem is only used when converting the TTP to a PartialOrderPlan, with the already implemented function _to_partial_order_plan.

@nicola-gigante
Copy link

Hi @anissaab, thanks for answering!

I'm not familiar with how you extract the STN, but I suppose you need to compare the plan with problems' the durative actions, right?

But, in a HierarchicalProblem, there are some constraints that are not represented in the actions but come from the task network.

Suppose that two durative actions A1 and A2 implement two subtasks T1 and T2, and we have an ordering constraint T1 < T2. Then, A1 will come before A2 in the TTP. Now, even if the durative actions themselves do not have any conflicting conditions, the STN extracted from the TTP has to still guarantee that A2 starts after the end of A1.

Is this something you can handle already? Or am I making things more complex than needed?

@anissaab
Copy link
Collaborator Author

anissaab commented Dec 6, 2023

Hi @anissaab, thanks for answering!

I'm not familiar with how you extract the STN, but I suppose you need to compare the plan with problems' the durative actions, right?

But, in a HierarchicalProblem, there are some constraints that are not represented in the actions but come from the task network.

Suppose that two durative actions A1 and A2 implement two subtasks T1 and T2, and we have an ordering constraint T1 < T2. Then, A1 will come before A2 in the TTP. Now, even if the durative actions themselves do not have any conflicting conditions, the STN extracted from the TTP has to still guarantee that A2 starts after the end of A1.

Is this something you can handle already? Or am I making things more complex than needed?

Indeed, for now the algorithm currently does not take into consideration task precedence. But it shouldn't be complicated to insert it for HierarchicalProblem !

@nicola-gigante
Copy link

Ok, thanks! It would be nice to have support for HierarchicalProblems as part of this PR! Let me know if I can help somehow.

@mikand
Copy link
Member

mikand commented Dec 14, 2023

@nicola-gigante Handling HTN is a cool idea! I would keep the scope of this PR to "flat" problems, but it is surely an interesting extension! If you have some CPU to work on this, maybe you can start a new PR (I would suggest to start from the non-temporal case and work on the partial order plan construction). I am sure @arbimo will have nice suggestions! 😄

@nicola-gigante
Copy link

Hi @mikand!

Yes, I have some CPU do work on this :)

@arbimo is my intuition correct that to support a HierarchicalProblem we only need to refine the STN to account for task precedence constraints?

@arbimo
Copy link
Member

arbimo commented Dec 15, 2023

@nicola-gigante Yes that is correct. The only (minor) annoyance being that constraints are expressed between a mix of abstracts and primitive tasks, so there is some transitive reasoning to do.

The TaskNetwork class provides some method to extract the temporal constraints and is a good entry point. Note that a "temporal constraints" is just a boolean expression that refers to a timepoint. It might be nested into others expression. The total-order and partial-order cases (and corresponding problem kinds) are the cases you probably want to focus first on. It should be easy to add a TASK_ORDER_STN kind that would require temporal constraints to form an STN (in the most expressive model they could for instance form a DTP).

@alvalentini alvalentini merged commit 4fcf49f into master Jan 15, 2024
8 checks passed
@alvalentini alvalentini deleted the ttp-to-stn branch January 15, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants