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

Update domain.pddl #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update domain.pddl #1

wants to merge 1 commit into from

Conversation

jzylf
Copy link

@jzylf jzylf commented Jun 7, 2018

67行与原71行重复

67行与原71行重复
@pluehne pluehne self-requested a review June 7, 2018 14:07
@pluehne pluehne self-assigned this Jun 7, 2018
Copy link
Member

@pluehne pluehne left a comment

Choose a reason for hiding this comment

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

@jzylf: Thank you so much for your contribution! Well spotted, this line is indeed redundant.

My unwritten policy is that the original domains and instances should not be modified, because I consider them the ground truth, and I don’t want them to be semantically altered. (I want the domains and instances in this repository to exactly represent what was run at the original competitions.)

With that said, I think that your modification is well justified. After all, the problem’s semantics aren’t changed at all.

What I’d suggest, though, is to add a note saying

Duplicate (power_on ?i) entries in conjunctive action preconditions were removed.

to the domain’s readme.

Aside from that, note that this duplicate entry is also present in the following domain variants:

To sum it up, I’d like to request the following changes:

  • repeat your change for the other domain variants listed above
  • add the note that I suggested above to all those domain’s readme files identically
  • please give your commits a meaningful English commit message

I’d be glad if you would address my suggestions 😃.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants