Problem about mirror_inds["sideneg_obs_inds"] / mirror_inds["sideneg_act_inds"] in sym_envs.py

[suppose]

Hi,

I'm a little confused about how to get mirrored obs/acts.
If I understand correctly, mirror_inds["sideneg_obs_inds"] should be the the index of obs that belongs to left or right side of a robot that need to be negated and swap when mirroring.
My problem is that only the obs of left side is included in mirror_inds["sideneg_obs_inds"] in the code, should be the corresponding right side also be included?
For example:
the mirroring function should fulfill obs = M(M(obs)).
if we have two corresponding left and right obs, say ob[left] = 1 and ob[right] = 2 and only "left" is in mirror_inds["sideneg_obs_inds"], after first mirroring ob[left] = 2 and ob[right] = -1, and mirroring them again, ob[left] = -1 and ob[right] = -2. It seems not correct. same thing for the action mirroring.

Looking forward to your responding!

[farzadab]

Hmm, I haven't thought about this in a long time but you might be right. I did a quick check to see if this was handled in code (i.e. you can apply the side-neg once before swapping and once after), but I don't see that either. Another possibility is also that I forgot something in how we defined side-neg.

@belinghy do you have any ideas?

[belinghy]

I'm not entirely sure because it's been so long. I think there was a reason to have separate neg_act_inds and sideneg_act_inds. Maybe it's related to this line https://github.com/UBCMOCCA/SymmetricRL/blob/master/symmetry/env_utils.py#L119.

[belinghy]

After more thought, I think you're right that sideneg should include both left and right indices.

I think the original intention of having sideneg is to handle the case where the character model has the same axis of rotation on both left and right counterparts. This causes the mirrored pose to have an extra negative sign on one side. For example, if +30 degree bends the right elbow inward, then it'll bend the left elbow outward if the axes of rotation are the same. So I think sideneg contains only the indices of one side for this reason. But this is wrong for mirroring.

sideneg is used only for NET and PHASE methods. The mistake basically causes the axis definitions, in the mirrored state and action space, to be different. For NET, the network should be able to handle this discrepancy. For PHASE, this is more problematic because it causes a bigger discontinuity when the phase changes, on top of the left-right swap.

Not sure if this makes any sense. I'm sure @farzadab will correct me if I missed something. 😄

[farzadab]

Yeah I'll still need to spend more time on it, this is when I really hope I had better comments in the first place 😅

The short answer is that for a normal mirroring function you don't need to differentiate between neg_obs and sideneg_obs anyway. As Ben pointed out, one reason that necessitated this was to simplify the NET architecture: the idea is that if we explicitly apply this to the actions and observations for example, the network can assume that both sides are swappable. Otherwise the network needed to handle 4 cases (constant, negated, left-right swapped, left-right negated) instead of 3 (all except the last one).
The reason I'm a bit hesitant is that I want to make sure we didn't make any mistakes when trying to handle everything at the same time (both the NET-compatible representation and the normal mirroring representation).

[suppose]

Thanks for your answer, I think for LOSS and DUP method, adding both side to sideneg would be correct. But not sure about NET and PHASE, haven't looked into that part of code.