Skip to content

Updated the domprob parser.#28

Merged
haz merged 6 commits intomainfrom
issue-25
Mar 20, 2026
Merged

Updated the domprob parser.#28
haz merged 6 commits intomainfrom
issue-25

Conversation

@haz
Copy link
Copy Markdown
Contributor

@haz haz commented Jan 18, 2026

Closes #25

@ssardina
Copy link
Copy Markdown
Collaborator

ssardina commented Mar 19, 2026

Checking this, finally! I spotted two side glitches, fixed them in c46d65e here directly, too much hassle to open issues for that; hope that's OK with you @haz

Now I am running the tests; det succeeds all, but normalization seems to be failing; checking that now:

image

@haz
Copy link
Copy Markdown
Contributor Author

haz commented Mar 19, 2026

Welcome back! Totally fine to commit directly on. 25a915e fix the issues?

@ssardina
Copy link
Copy Markdown
Collaborator

OK @haz , I am almost done. I need to fix the test cases bc the new pddl system seems to output things different than before. For example, it adds object type to all non-typed constants:

image

This made me think our comparison between two PDDLs whether they are equal could be too strong and fragile. As soon as there is a char difference, a test will fail. So any re-formatting of the PDDL would break comparison between two otherwise same PDDL files.

I am thinking whether I should change the unit test cases to not care about the exact formatting of a PDDL, whether it has a space here or two or 10, similar with \n

Will fix and improve this tomorrow and get this out of the way. Thanks for the patient Chris! 👍🏽

@haz
Copy link
Copy Markdown
Contributor Author

haz commented Mar 19, 2026

Ya, I'd say skip spacing for comparisons would be best. Let the pddl package be finicky with that

@ssardina
Copy link
Copy Markdown
Collaborator

ssardina commented Mar 20, 2026

The normalizer is dropping the :non-deterministic requirement, but it should not. The normalized PDDL would still use oneof:

image

I am puzzled why @haz decided not to include that requirement, there is explicit code (see screenshot) to drop it. Maybe it was wrongly imported from a previous determinizer?

@ssardina
Copy link
Copy Markdown
Collaborator

OK good, now all tests are passing:

image
  • Sync the expected files to what the new pddl lib gives (e.g., adds - object)
  • made equivalence between 2 pddls independent of syntax in expected file by parsing and printing via pddl lib

Copy link
Copy Markdown
Collaborator

@ssardina ssardina left a comment

Choose a reason for hiding this comment

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

OK checked it all and looks good now. Fixed and improved a few things, including aligning all tests for determinizer and normalizer.

All good to go Chris @haz !

Comment thread fondutils/pddl/domprob.py Outdated
Comment thread fondutils/pddl/domprob.py
Comment thread fondutils/pddl/domprob.py
Comment thread fondutils/pddl/domprob.lark Outdated
@haz
Copy link
Copy Markdown
Contributor Author

haz commented Mar 20, 2026

Ya, that requirements removal was an oversight, and almost certainly a copy/paste from the determinizer [here]. Nice catch, and thanks for the review!

@haz haz merged commit f0880ba into main Mar 20, 2026
@ssardina ssardina deleted the issue-25 branch March 20, 2026 21:27
@ssardina
Copy link
Copy Markdown
Collaborator

ssardina commented Mar 20, 2026

@haz , should we bump the version now that is aligned with the new pddl?

I was going to do it to v0.1.8 , but I thought I would check with you. I think we may want to do v0.2.0 as they are changes based on a new pddl and will not be "backwards" compatible

@haz
Copy link
Copy Markdown
Contributor Author

haz commented Mar 20, 2026

Ah, good point. Let's go with 0.2.0, as it'll be a bit broken with the update.

@ssardina
Copy link
Copy Markdown
Collaborator

Done! upgraded to 0.2.0

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.

Check and align with recent pddl changes

2 participants