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

Validator Tracing and TimeTriggeredValidator #524

Merged
merged 15 commits into from
Jan 15, 2024
Merged

Conversation

mikand
Copy link
Member

@mikand mikand commented Nov 17, 2023

In this PR we extend the internal sequential validation engine to return the sequence of states encountered by executing a sequential plan and we add support for validation (and tracing) of TimeTriggeredPlans

@mikand mikand marked this pull request as draft November 17, 2023 16:57
@karpase
Copy link
Collaborator

karpase commented Nov 19, 2023

Commenting just to say I would like to see this implemented - I have some experiments this could really help with.

@Framba-Luca
Copy link
Contributor

In pr #509 also happens a "transformation" of a temporal structure into a non-temporal one. After the merge of both there will most certainly be space for refactoring (that maybe can be considered also when designing the missing code).

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2023

Codecov Report

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

Comparison is base (da8ab70) 84.96% compared to head (2567ee5) 84.92%.
Report is 16 commits behind head on master.

Files Patch % Lines
unified_planning/engines/plan_validator.py 83.80% 34 Missing ⚠️
unified_planning/test/test_plan_validator.py 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   84.96%   84.92%   -0.04%     
==========================================
  Files         200      200              
  Lines       26433    26611     +178     
==========================================
+ Hits        22458    22600     +142     
- Misses       3975     4011      +36     

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

@alvalentini alvalentini marked this pull request as ready for review December 1, 2023 14:12
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.

LGTM! 😃

I left 4 comments. 2 are minor code-level improvements, 1 I think it's a bug, the 4th one is just a note on the SequentialValidator supported_kind

unified_planning/engines/plan_validator.py Outdated Show resolved Hide resolved
unified_planning/engines/plan_validator.py Show resolved Hide resolved
unified_planning/engines/plan_validator.py Outdated Show resolved Hide resolved
unified_planning/engines/plan_validator.py Outdated Show resolved Hide resolved
Copy link
Member

@arbimo arbimo left a comment

Choose a reason for hiding this comment

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

LGTM! I just added a small comment about an awkwardness in error handling.

unified_planning/engines/plan_validator.py Outdated Show resolved Hide resolved
@arbimo arbimo merged commit e66c6d3 into master Jan 15, 2024
8 checks passed
@arbimo arbimo deleted the validation-time-and-traces branch January 15, 2024 09:56
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

6 participants