Support static-body derived predicates#2
Merged
Merged
Conversation
What: - New ground/derived.rs module with DerivedRule, eval_gd, and expand_into_init - build_fact_universe emits derived head groundings alongside base predicates - expand_into_init evaluates rule bodies against init facts and materialises truths - eval_gd handles and, or, not, imply, exists, forall, equality, and fluent comparison rejection - 4 unit tests (happy-path, exists+equality, fluent-body rejection, recursion rejection) - Integration test asserting plan length 7 on hanoi-derived.pddl - New hanoi-derived.pddl example using is-peg and smaller as static rule bodies Why: - Support :derived-predicates PDDL requirement without runtime cost - Static bodies are computed once at ground time; search and heuristics stay untouched Findings: - derived heads not listed in :predicates still get groundings via build_fact_universe extension - add_derived_fact must set init bits for facts already emitted by build_fact_universe
There was a problem hiding this comment.
Pull request overview
Adds support for PDDL :derived-predicates whose bodies are static, by evaluating them once during grounding and materializing the resulting derived facts into the initial state.
Changes:
- Introduces
ground/derived.rsto collect/validate derived rules and evaluate derived bodies (eval_gd) against the init fact set. - Extends fact-universe construction to include groundings for derived predicate heads, and expands derived facts into
Task.initduring grounding. - Adds a Hanoi derived-predicate example plus unit/integration tests to validate optimal plan length and validation behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
crates/miniplan/src/ground/derived.rs |
New derived-rule collection/validation, boolean GD evaluator, and init expansion logic |
crates/miniplan/src/ground/action.rs |
Calls derived init expansion and adds derived head groundings to the fact universe |
crates/miniplan/src/ground/formula.rs |
Exposes term_to_string for derived evaluation and documents quantifier limitations of DNF walking |
crates/miniplan/src/ground.rs |
Registers the new derived module |
examples/pddl/hanoi-derived.pddl |
New example domain/problem using a derived can-place predicate |
crates/miniplan/tests/hanoi_derived.rs |
New integration test asserting optimal plan length (7) |
README.md |
Updates feature matrix to mention derived predicates (static bodies) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Before: - Derived predicate head name collision with :predicates rejected valid domains - Multi-layer validation only checked rules collected so far - Fluent extraction collected from effect conditions, not just primitive effects - Forall handling ignored nested When/Forall in conditional effects - Quantifier bindings appended to outer bindings, causing shadowing bugs - Derived rules collected twice: in build_fact_universe and expand_into_init - hanoi-derived.pddl had type mismatch in smaller predicate After: - Remove collision check; derived heads may appear in :predicates (standard PDDL) - First pass collects all derived head names before validating - extract_fluent_names only inspects primitive effects - Recursive collect_pred_names_in_conditional_effect handles all variants - ext_bindings prepended before bindings so inner bindings override - Derived rules collected once in ground() and passed to both consumers - smaller predicate accepts (disc, location) to match derived rule body Why: - Address all 8 PR review comments on PR #2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Support PDDL
:derived-predicateswith static bodies — derived facts are computed once at ground time and materialised as regular facts in the initial state. No runtime, search, or heuristic code changes needed.Before
StructureDef::Derivedwas silently ignored by the grounder. Domains using:derived-predicatesloaded but derived rules had no effect.After
ground/derived.rsmodule withDerivedRule,DerivedRuleSet,collect,eval_gd, andexpand_into_initbuild_fact_universeemits derived head groundings alongside base predicates so action preconditions resolve fact IDsexpand_into_initevaluates each rule body against the init fact set and sets matching derived facts trueeval_gdis a dedicated boolean evaluator that understandsand,or,not,imply,exists,forall, equality, and rejects fluent comparisonsexamples/pddl/hanoi-derived.pddlwith optimal plan length 7, usingis-pegandsmalleras static rule bodiesOutcome
All 90 tests pass (43 unit + 21 integration + 33 doctests). Existing
hanoi.pddlbehaviour is untouched.Findings
:predicatesstill get their full Cartesian product of groundings via thebuild_fact_universeextension, so action preconditions referencing them resolve correctly.add_derived_factmust handle facts that were already emitted bybuild_fact_universe(just set the init bit) as well as entirely new facts.onare rejected. The Hanoi-derived example works becausecan-placedepends only onis-pegandsmaller, both static.Review Instructions
ground/derived.rs—collectvalidation logic,eval_gdquantifier handling, andexpand_into_initflowground/action.rschanges —build_fact_universeextension and insertion ofexpand_into_initcall between init build and action groundingexamples/pddl/hanoi-derived.pddlsolves to plan length 7 with the integration testhanoi.pddltests)