search

snap-research / articulated-animation

Code for Motion Representations for Articulated Animation paper
https://snap-research.github.io/articulated-animation/
Other
1.21k stars 348 forks source link

Question about revert_axis_swap #13

Open enhuiz opened 3 years ago

enhuiz commented 3 years ago

I have a small question related to the revert_axis_swap operation, I notice you flip the sign of the affine matrix when the first element is negative, what are the benefits of this?

AliaksandrSiarohin commented 3 years ago

When you compute pca orientation of the axis could be arbitrary. For example x axis in canonical space may point left or right. If the orientation is different in the source and driving, corresponding region will be flipped.

enhuiz commented 3 years ago

Thanks for your reply. Does this only prevent the horizontal flip? It seems only the first basis is considered.

https://github.com/snap-research/articulated-animation/blob/66979bea61ddd8166c13008d5b603740ea105cd5/modules/model.py#L211

For preventing vertical flip, do we also need to do value = value * torch.sign(value[:, :, 1:2, 1:2])?

AliaksandrSiarohin commented 3 years ago

I did not observe problems with other flip. So I did not include this, this probably depends on actual svd implementation.

enhuiz commented 3 years ago

I have encountered some strange flips even with value = value * torch.sign(value[:, :, 0:1, 0:1]).

Yes, it should relate to the svd implementation. Just found torch.svd (PyTorch 1.8.1, not sure whether other versions have this problem) does not guarantee that the determinant of u always has the same sign.

The following workaround together with value = value * torch.sign(value[:, :, 0:1, 0:1]) solved my problem:

# flip the first basis when det < 0
det = torch.linalg.det(u).unsqueeze(-1).unsqueeze(-1)
u = u * torch.sign(torch.cat([det, det.abs()], dim=-1))