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
358 stars 158 forks source link

An instance with new type could be added to the KB but it should not #304

Open dgerod opened 3 years ago

dgerod commented 3 years ago

While working with KB, I have a conceptual doubt.

There is domain file that defines 'place', 'bin' and other types but not 'robot', and this file is loaded by the Knowledge Base when it is started. Now I add the following instances:

All the instances are dded to the Knowledge Base, also 'r' that was not defined in the domain file. I see the following trace:

Node: /rosplan_knowledge_base
Time: 10:43:13.422947385 (2021-08-24)
Severity: Info
Published Topics: /rosout, /rosplan_knowledge_base/status/update

KCL: (/rosplan_knowledge_base) Adding instance (robot, r)

Location:
/home/ubuntu/ros_ws/src/rosplan/rosplan_knowledge_base/src/KnowledgeBase.cpp:KnowledgeBase::addKnowledge:383 

Although 'r' is not see in RQT application for ROSPlan, I could retrieve the instance from the KB.

image

I have a code in which I retrieve all the instances of the KB including their types, see here. When I execute it, all instance are retrieved with the type except 'r' that is returned without type. To retrieve the type of an instance I am using "get domain types" service of the Knowledge Base, which calls the following code:

bool 
PDDLKnowledgeBase::getTypes(
   rosplan_knowledge_msgs::GetDomainTypeService::Request  &req,
   rosplan_knowledge_msgs::GetDomainTypeService::Response &res) 
{
...
}

At this moment I am a bit confused, for me it is not clear what is the expectation of this situation. And I think the behavior is not coherent. So, I have several questions:

gerardcanal commented 3 years ago

Hi Diego, That's an interesting issue. I am not sure about the goal or use-case you are trying though. ROSplan currently doesn't regenerate domains, so adding types to the KB would probably cause issues with the planner and generated problem file. First, to answer your questions:

Does KB accept instances of non existing type in its domain?

It apparently does, based on your examples. To the question of "should it accept instances of non-existing type" I would say it shouldn't, and this is a bug.

Why these instances are managed different that the ones which type is defined in the domain? They are not shown in the same place and the type is not stored in the domain information with others.

I would need to look into detail, but my guess here is that in RQT for instance it gets all the types, and then it gets the instances of each type. As it doesn't have the type robot, it can't find r.

In case that current behavior is correct, how should I get the types of all instances without considering if they are part of the domain or not?

If you call /rosplan_knowledge_base/state/instances with an empty type name, I believe it should return all the instances regardless of type.

How a new type of an instance could be added without modifying the domain file?

That is the use-case I am not sure I can see. The planner will plan with the domain that the knowledge base receives, which doesn't have the robot type. When you add an instance of another type, it will then appear in the problem file generated by the problem generator, but not in the domain, which will likely result in the planner giving an error.

I believe this is why there's no service to add types because we are not generating the domain file, so adding a type would create this inconsistency between the knowledge base and the domain it was instantiated from.

Pinging @m312z as he'll probably can give more about the reasons for all this and correct me if I'm wrong.

m312z commented 3 years ago

The current knowledge base expects the state to change, but the domain to remain static. In the implementation, the state is contained within a set of c++ data types and the domain is retrieved from VAL's data structures as required (see ptree.h).

Gerard is correct that in many places the types are fetched, then each instance of those types. Also that an object of a non-existant type would crash the planners. In any case, if the domain is using types, then the new object's type would not match any of the parameter types for the operators, and so should be unusable.

Modifying the domain would be a very useful feature. Then the domain and problem could be auto-generated together.

dgerod commented 3 years ago

Thanks both for yours quick answers!

I would summarize them in two points:

Please, could you confirm my understanding?

About my use case, I don't have any, I have arrived to this situation due to a wrong modification of my domain file :-).

gerardcanal commented 3 years ago

Hi Diego, Yes, I agree that's a good summary, and agree this is a bug and the type should be checked. Added the bug label. If you wanna PR it feel free, otherwise we'll try to take on this (hopefully) soon.