DexterousHands icon indicating copy to clipboard operation
DexterousHands copied to clipboard

Possible mismatch between paper and code?

Open sheffier opened this issue 2 years ago • 4 comments

Hi,

In your paper, you detail the following action space for the BottleCap task:

Screen Shot 2022-07-27 at 10 22 50

As can be seen in the attached table, the indices for ShadowHand base translation & rotation are:

  • Right hand: 20-25
  • Left hand: 46-51

In the task's python file, the docstring of the pre_physics_step method agrees with the above table:

        """
        The pre-processing of the physics step. Determine whether the reset environment is needed, 
        and calculate the next movement of Shadowhand through the given action. The 52-dimensional 
        action space as shown in below:
        
        Index   Description
        0 - 19 	right shadow hand actuated joint
        20 - 22	right shadow hand base translation
        23 - 25	right shadow hand base rotation
        26 - 45	left shadow hand actuated joint
        46 - 48	left shadow hand base translation
        49 - 51	left shadow hand base rotation

        Args:
            actions (tensor): Actions of agents in the all environment 
        """

However, it seems that you used different indices in the code:

self.apply_forces[:, 1, :] = actions[:, 0:3] * self.dt * self.transition_scale * 100000
self.apply_forces[:, 1 + 26, :] = actions[:, 26:29] * self.dt * self.transition_scale * 100000
self.apply_torque[:, 1, :] = self.actions[:, 3:6] * self.dt * self.orientation_scale * 1000
self.apply_torque[:, 1 + 26, :] = self.actions[:, 29:32] * self.dt * self.orientation_scale * 1000

From the above code snippet, it seems that the actual indices that are used are:

  • Right hand: 0-5
  • Left hand: 26-31

Is it just a mismatch between the paper and the implementation or am I missing something?

sheffier avatar Jul 27 '22 07:07 sheffier

Hi,

In your paper, you detail the following action space for the BottleCap task:

Screen Shot 2022-07-27 at 10 22 50

As can be seen in the attached table, the indices for ShadowHand base translation & rotation are:

  • Right hand: 20-25
  • Left hand: 46-51

In the task's python file, the docstring of the pre_physics_step method agrees with the above table:

        """
        The pre-processing of the physics step. Determine whether the reset environment is needed, 
        and calculate the next movement of Shadowhand through the given action. The 52-dimensional 
        action space as shown in below:
        
        Index   Description
        0 - 19 	right shadow hand actuated joint
        20 - 22	right shadow hand base translation
        23 - 25	right shadow hand base rotation
        26 - 45	left shadow hand actuated joint
        46 - 48	left shadow hand base translation
        49 - 51	left shadow hand base rotation

        Args:
            actions (tensor): Actions of agents in the all environment 
        """

However, it seems that you used different indices in the code:

self.apply_forces[:, 1, :] = actions[:, 0:3] * self.dt * self.transition_scale * 100000
self.apply_forces[:, 1 + 26, :] = actions[:, 26:29] * self.dt * self.transition_scale * 100000
self.apply_torque[:, 1, :] = self.actions[:, 3:6] * self.dt * self.orientation_scale * 1000
self.apply_torque[:, 1 + 26, :] = self.actions[:, 29:32] * self.dt * self.orientation_scale * 1000

From the above code snippet, it seems that the actual indices that are used are:

  • Right hand: 0-5
  • Left hand: 26-31

Is it just a mismatch between the paper and the implementation or am I missing something?

Hi @sheffier ,

Yes you are right, this is a mismatch between paper and code, the correct indices for ShadowHand base translation & rotation are:

  • Right hand: 0-5
  • Left hand: 26-31

In fact, there may be some tiny mismatches between the paper and code, which are caused by careless writing or code version problems. I'm very sorry for these mismatches, but they basically don't affect the task very much. Since the papers are still under review, I will fix them in the rebuttal stage.

I suggest that you can now mainly refer to the actual code implementation if you encounter the mismatch issues. If you have any other questions, I very much welcome you to continue to ask us :-).

Hope this can help you.

cypypccpy avatar Jul 27 '22 09:07 cypypccpy

Hi @cypypccpy

As always, thanks for the quick and detailed answer :)

sheffier avatar Jul 27 '22 09:07 sheffier

Hi,

For "shadow_hand_lift_underarm.py" the indices are reused in different places which will lead to wrong forces and torques. (Other tasks seem okay)

https://github.com/PKU-MARL/DexterousHands/blob/f5d55051a957812b80ef7f5a6317b0413113d09c/bidexhands/tasks/shadow_hand_lift_underarm.py#L1076-L1095

The indices in L1076, L1083 and L1093 should be changed to [6:26], [32:52] and [26:29] respectively to be compliant with the current implementation of other tasks.

Relento avatar Feb 12 '23 02:02 Relento

Hi @Relento ,

Thank you very much for pointing it out! Sorry, this is a very big mistake due to my negligence. Yes, it should be changed to [6:26], [32:52], and [26:29]. I've fixed it in the latest commit.

Thanks a lot.

cypypccpy avatar Feb 15 '23 02:02 cypypccpy