KCL-Planning / ROSPlan

The ROSPlan framework provides a generic method for task planning in a ROS system.
http://kcl-planning.github.io/ROSPlan
BSD 2-Clause "Simplified" License
353 stars 159 forks source link

Initial state and goals are incorrectly generated in problem file by PDDLProblemGenerator #253

Open dgerod opened 4 years ago

dgerod commented 4 years ago

Now that I am moving my code from using old ROSPlan version to the latest one, I have realized about this issue.

The "makeInitialState()" method of "PDDLProblemGenerator" writes parameters of a predicate following the order in which they are stored in KB, but this is wrong. And same problem is happening in "makeGoals()". The parameters of a predicate could be stored in any order in the KB because they have name, but the problem file generated is wrong due to this assumption.

My domain:

(define (domain organize_items)
(:requirements :typing :disjunctive-preconditions)
(:types
    place
    object
    container - place
    item - object
    box - place
)
(:predicates
    (robot-at ?p - place)
    (object-at ?o - object ?p - place)
    (localised ?o - object)
    (grasped ?o - object)
    (free-tool)
)
... MORE_CODE_HERE ...
)

The correct generated problem file should be:

(define (problem task)
(:domain organize_items)
(:objects
    home - place
    c1 - container
    item_0 item_2 item_1 item_3 - item
    b1 - box
)
(:init
    (robot-at home)
    (object-at item_0 c1)
    (object-at item_2 c1)
    (object-at item_1 c1)
    (object-at item_3 c1)
    (free-tool)
)
(:goal (and
    (object-at item_0 b1)
    (object-at item_2 b1)
    (object-at item_1 b1)
    (object-at item_3 b1)
))
)

Generated problem file:

(:init
    (robot-at home)
    (object-at c1 item_0)
    (object-at c1 item_2)
    (object-at c1 item_1)
    (object-at c1 item_3)
    (free-tool)
)
(:goal (and
    (object-at b1 item_0)
    (object-at b1 item_2)
    (object-at b1 item_1)
    (object-at b1 item_3)
))

As this is is not happening in the old version of ROSPlan, I have compared both codes. The makeInitialState() in the new version of ROSPlan is:

std::vector<rosplan_knowledge_msgs::DomainFormula>::iterator ait = domainAttrSrv.response.items.begin();
for(; ait != domainAttrSrv.response.items.end(); ait++) {

    rosplan_knowledge_msgs::GetAttributeService attrSrv;
    attrSrv.request.predicate_name = ait->name;
    if (!getPropsClient.call(attrSrv)) {
        ROS_ERROR("KCL: (PDDLProblemGenerator) Failed to call service %s: %s", state_proposition_service.c_str(), attrSrv.request.predicate_name.c_str());
    } else {

        for(size_t i=0;i<attrSrv.response.attributes.size();i++) {

            rosplan_knowledge_msgs::KnowledgeItem attr = attrSrv.response.attributes[i];

            pFile << "    (";       

            //Check if the attribute is negated
            if(attr.is_negative) pFile << "not (";

            pFile << attr.attribute_name;
            for(size_t j=0; j<attr.values.size(); j++) {
                pFile << " " << attr.values[j].value;
            }
            pFile << ")";

            if(attr.is_negative) pFile << ")";

            pFile << std::endl;
        }
    }
    pFile << std::endl;

    ... MORE_CODE_HERE ...
}

While in the old version there is the code after "// find the PDDL parameters in the KnowledgeItem". And in my understanding it is this code which guarantees to write the parameters of the predicates in the correct order.

// add knowledge to the initial state
for(size_t i=0; i<environment.instance_attributes.size(); i++) {

    std::stringstream ss;
    bool writeAttribute = false;

    // check if attribute is a PDDL predicate
    std::map<std::string,std::vector<std::string> >::iterator ait;
    ait = environment.domain_predicates.find(environment.instance_attributes[i].attribute_name);
    if(ait != environment.domain_predicates.end()) {
        writeAttribute = true;
        ss << "    (" + environment.instance_attributes[i].attribute_name;

        // find the PDDL parameters in the KnowledgeItemAs conclusion, in 
        for(size_t j=0; j<ait->second.size(); j++) {
            bool found = false;
            for(size_t k=0; k<environment.instance_attributes[i].values.size(); k++) {
                if(0 == environment.instance_attributes[i].values[k].key.compare(ait->second[j])) {
                    ss << " " << environment.instance_attributes[i].values[k].value;
                    found = true;
                }
            }
            if(!found) writeAttribute = false;
        };
        ss << ")";
    }
    if(writeAttribute) pFile << ss.str() << std::endl;
}

Same code could be found in "makeGoals()" of old version of ROSPlan.

Therefore, my conclusion is that this is a bug. Please, could someone confirm it? Thanks.

dgerod commented 4 years ago

Working on this, see: https://github.com/dgerod/ROSPlan/tree/feature/Fix-makeInitialState-and-makeGoals-of-PDDLProblemGenerator