[BUG] Negative effects are grounded as positive

[o-fir]

Using PDDL4J from DEVEL branch. Latest build.

Given a definition of a close_door action like this:

(:action close_door
		:parameters (?cd_rv - Regular_Vehicle)
		:precondition 
			(and
				(Door_Open ?cd_rv)
			)
		:effect
			(and
				(not (Door_Open ?cd_rv))
			)
	)

a grounded action (close_door flugzeug) has (door_open flugzeug) in its positive preconditions instead of negative.

Domain (UM-Translog) and problem are attached below.
domainTranslog.txt
p02.txt

[o-fir]

After enabling the logger, I can safely say that the problem is somewhere in the problem.instantiate() from DefaultProblem.java

    public final void instantiate() {
        this.initialization();
        this.preinstantiation();
        this.instantiation();
        this.postinstantiation();
        this.finalization();
    }

this.initialization() is correct, because the logger prints:

Action close_door
Instantiations:
?X0 - Regular_Vehicle : ?
Preconditions:
(Door_Open ?X0)
Effects:
(not (Door_Open ?X0))

There are no problems at the end of this.instantiation() either, since logger prints:

Action close_door
Instantiations:
?X0 - Regular_Vehicle : Flugzeug
Preconditions:
(Door_Open Flugzeug)
Effects:
(not (Door_Open Flugzeug))

Similarly, in this.postinstantiation() when logger outputs actions simplified based on ground inertia:

Action close_door
Instantiations:
?X0 - Regular_Vehicle : Flugzeug
Preconditions:
(Door_Open Flugzeug)
Effects:
(not (Door_Open Flugzeug))

However, during this.finalization(), the close_door action is wrong:

Action close_door
Instantiations:
?X0 - Regular_Vehicle : Flugzeug
Preconditions:
(and (Door_Open Flugzeug))
Effects:
(and (Door_Open Flugzeug))

I will check if there's a bug in finalizeActions() called inside finalization()

---

finalizeActions() from FinalizedProblem.java applies 2 functions on each instantiated action (IntAction):

1. IntAction normalizeAction()
2. Action finalizeAction()

After normalizing close_door, its effects are (not (15 0)).
After calling finalizeAction() on normalized close_door, the effects (now bitvector) are as follows:

** Positive fluents **
{12}
** Negative fluents **
{}

[o-fir]

I believe I have found the source of a bug:
This block in function finalizeAction inside problem.FinalizedProblem.java.

// Initialize the effects of the action
        final LinkedList<Expression<Integer>> effects = new LinkedList<>();
        if (action.getEffects().getConnector().equals(Connector.ATOM)) {
            effects.add(action.getEffects());
        } else {
            effects.addAll(action.getEffects().getChildren());
        }

Recall that close_door resembles this:

(:action close_door
  :parameters (?X0 - REGULAR_VEHICLE)
  :precondition
  (door_open ?X0)
  :effect
  (and
    (not (door_open ?X0)))
)

action.getEffects() is (not (15 0)) with connector NOT. Because connector is not ATOM, we end up using this case effects.addAll(action.getEffects().getChildren()); and effects are populated with (15 0) with connector ATOM. In other words, connector NOT` is thrown away.

To test this assumption I have changed the domain definition to this, where negative effect is duplicated:

(:action close_door
		:parameters (?cd_rv - Regular_Vehicle)
		:precondition 
			(and
				(Door_Open ?cd_rv)
			)
		:effect
			(and
				(not (Door_Open ?cd_rv))
				(not (Door_Open ?cd_rv))
			)
	)

And the problem gets grounded properly

[o-fir]

As a solution I propose changing the block from this

        if (action.getEffects().getConnector().equals(Connector.ATOM)) {
            effects.add(action.getEffects());
        } else {
            effects.addAll(action.getEffects().getChildren());
        }

to this

        if (action.getEffects().getConnector().equals(Connector.AND)) {
            effects.addAll(action.getEffects().getChildren());
        } else {
            effects.add(action.getEffects());
        }

I will do some tests and if everything works out I will prepare a merge.

[christopherlumsden]

Thank you very much for sharing your findings @o-fir 🙇‍♂ I hit the same snag!

I believe checking for AND would not capture disjunctive expressions that use OR. Perhaps it would be better to be really explicit and check it is a literal? i.e. it is either an atom or a negated atom?

if (action.getEffects().isLiteral()) {
    effects.add(action.getEffects());
} else {
    effects.addAll(action.getEffects().getChildren());
}

Did you end up doing any further testing on this?

Edit: I discovered an isLiteral() method already exists, so removed some code that was performing the same check.

[o-fir]

I fixed the issue, but I see that I forgot to push changes. I'll do it next week asap, as I'm on vacation right now.