rll / lfd

LfD: Learning from Demonstrations for Robotic Manipulation
Other
46 stars 16 forks source link

add_objects now assumes that a robot has being added #3

Closed alexlee-gk closed 9 years ago

alexlee-gk commented 9 years ago

Commit 58d42aa36dfdf1385171685a16c52e965d188a9d started assuming in function add_objects that at least one of the objects being added is a robot: https://github.com/dhadfieldmenell/lfd/blob/58d42aa36dfdf1385171685a16c52e965d188a9d/core/simulation.py#L35

Previous behavior was to set the self.robot variable only if a robot was just added: https://github.com/dhadfieldmenell/lfd/blob/1fe2475d76c90be45202316f79bf79fa92c3bb2e/core/simulation.py#L34-L37

Can you revert the behavior?

dhadfieldmenell commented 9 years ago

Yup, that looks like a bug.

That said, the code is kinda hard to parse. The point is that we set self.robot if there is a robot being added and through an error if we try to add two. Maybe pull out an explicit variable for the condition?

ie if(robot_added & self.robot) raise ValueError elif (robot_added) init self.robot

On Tue Nov 11 2014 at 4:31:01 PM alexlee-gk notifications@github.com wrote:

Commit 58d42aa https://github.com/dhadfieldmenell/lfd/commit/58d42aa36dfdf1385171685a16c52e965d188a9d started assuming in function add_objects that at least one of the objects being added is a robot:

https://github.com/dhadfieldmenell/lfd/blob/58d42aa36dfdf1385171685a16c52e965d188a9d/core/simulation.py#L35

Previous behavior was to set the self.robot variable only if a robot was just added:

https://github.com/dhadfieldmenell/lfd/blob/1fe2475d76c90be45202316f79bf79fa92c3bb2e/core/simulation.py#L34-L37

Can you revert the behavior?

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3.

alexlee-gk commented 9 years ago

Yeah, that looks better.

Ideally we can move to an abstraction to have as many robots as we want, but that might not be of so much use because trajopt assumes one robot everywhere. That being said, dealing with an arbitrary number of robots would be beneficial. I could see ourselves using the kuka arm and the pr2 in a single setup. On Tue Nov 11 2014 at 6:47:19 PM Dylan Hadfield-Menell < notifications@github.com> wrote:

Yup, that looks like a bug.

That said, the code is kinda hard to parse. The point is that we set self.robot if there is a robot being added and through an error if we try to add two. Maybe pull out an explicit variable for the condition?

ie if(robot_added & self.robot) raise ValueError elif (robot_added) init self.robot

On Tue Nov 11 2014 at 4:31:01 PM alexlee-gk notifications@github.com wrote:

Commit 58d42aa < https://github.com/dhadfieldmenell/lfd/commit/58d42aa36dfdf1385171685a16c52e965d188a9d>

started assuming in function add_objects that at least one of the objects being added is a robot:

https://github.com/dhadfieldmenell/lfd/blob/58d42aa36dfdf1385171685a16c52e965d188a9d/core/simulation.py#L35

Previous behavior was to set the self.robot variable only if a robot was just added:

https://github.com/dhadfieldmenell/lfd/blob/1fe2475d76c90be45202316f79bf79fa92c3bb2e/core/simulation.py#L34-L37

Can you revert the behavior?

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3.

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3#issuecomment-62661408.

dhadfieldmenell commented 9 years ago

I think the right strategy is one 'robot' which may have multiple manipulators. Trajopt should be able to handle that reasonably well (although base planning makes some assumptions that could be problematic).

On Wed, Nov 12, 2014 at 2:05 PM, alexlee-gk notifications@github.com wrote:

Yeah, that looks better.

Ideally we can move to an abstraction to have as many robots as we want, but that might not be of so much use because trajopt assumes one robot everywhere. That being said, dealing with an arbitrary number of robots would be beneficial. I could see ourselves using the kuka arm and the pr2 in a single setup. On Tue Nov 11 2014 at 6:47:19 PM Dylan Hadfield-Menell < notifications@github.com> wrote:

Yup, that looks like a bug.

That said, the code is kinda hard to parse. The point is that we set self.robot if there is a robot being added and through an error if we try to add two. Maybe pull out an explicit variable for the condition?

ie if(robot_added & self.robot) raise ValueError elif (robot_added) init self.robot

On Tue Nov 11 2014 at 4:31:01 PM alexlee-gk notifications@github.com wrote:

Commit 58d42aa <

https://github.com/dhadfieldmenell/lfd/commit/58d42aa36dfdf1385171685a16c52e965d188a9d>

started assuming in function add_objects that at least one of the objects being added is a robot:

https://github.com/dhadfieldmenell/lfd/blob/58d42aa36dfdf1385171685a16c52e965d188a9d/core/simulation.py#L35

Previous behavior was to set the self.robot variable only if a robot was just added:

https://github.com/dhadfieldmenell/lfd/blob/1fe2475d76c90be45202316f79bf79fa92c3bb2e/core/simulation.py#L34-L37

Can you revert the behavior?

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3.

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3#issuecomment-62661408.

— Reply to this email directly or view it on GitHub https://github.com/dhadfieldmenell/lfd/issues/3#issuecomment-62803500.

alexlee-gk commented 9 years ago

That's definitely a good option.

In the mean time, @antingshen can you fix the assumption that you introduced? My example script register.py doesn't work otherwise.

alexlee-gk commented 9 years ago

Undo change in add_objects with commit e42d6b1fa7fc8e04ec833abd5b00e7370bef0ed3 since no one has fixed this yet.