Skip to content

Conversation

@marfvr
Copy link
Member

@marfvr marfvr commented Oct 5, 2025

Proposed changes

As noted in #150, in the domain parser, sometimes (like in derived predicates), we do not propagate typing annotations from its definition to its condition.

Fixes

Work in progress, fixes #150

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING doc
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

Further comments

I added tests to verify that variable types are handled correctly for action definitions, and it appears that this is the case.
This is because, whenever the LR parser encounters the action_parameters tag, it reinitializes the varname-to-Variable mapping:

self._current_parameters_by_name = {

To me it seems this is not easily doable in the case of derived_predicates:

pddl/pddl/parser/domain.py

Lines 139 to 143 in c381a08

def derived_predicates(self, args):
"""Process the 'derived_predicates' rule."""
predicate = args[2]
condition = args[3]
return DerivedPredicate(predicate, condition)
, since predicate parsing is also used in formula parsing, and we need to distinguish the cases when we have to reinitialize the mapping or not. Perhaps changing the grammar with derived_predicate_def instead of atomic_formula_skeleton might help!
derived_predicates: LPAR DERIVED atomic_formula_skeleton gd RPAR
; in that case, we could handle the derived predicate definition separately, by resetting the mapping.

With a recursive parser, it would have been handled more easily: simply reset the mapping whenever we start to parse a derived predicate. However, as seen in here, we have already parsed both the derived predicate definition and its formula definition. More generally, the difficulty lies in the fact that our parser is a bottom-up parser.

A solution would be to re-parse the data structures (e.g., the AST of the formula) and replace variables without types with the same variable but with the right types. It would not be a "one-pass" computation, though.

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.

Same variable considered different

2 participants