ROSPlan icon indicating copy to clipboard operation
ROSPlan copied to clipboard

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

Open dgerod opened this issue 4 years ago • 1 comments

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 avatar Jun 30 '20 06:06 dgerod

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

dgerod avatar Jul 11 '20 08:07 dgerod