Updated use of weights in procrustes analysis (update to #1647)

[tillns]

Hi Michael,

I noticed that the use of the weights parameter can lead to incorrect results, e.g., undesired shifts in the transformed points. I reviewed my own changes two years ago in #1647, and I noticed that I oversimplified the problem. Apparently, a procrustes analysis with non-uniform weights cannot be solved analytically but requires iterative optimization approaches. I updated the documentation and also changed the translation and scaling computation of b to also incorporate the weights to avoid some of the undesired effects. As it is now, the code should yield the optimal solution for uniform weights (as usual) and for binary weights; for the latter, I added some code to filter out zero-weighted points from the transform computations. For general non-uniform weights, the optimal translation and scaling are computed, but they're based on a possibly suboptimal rotation. If we wanted to find a better rotation, an iterative optimization approach would need to be added, but I was too lazy to do that haha

Cheers
Till

[mikedh]

Awesome thanks for the follow-up PR!! I've been really impressed with the performance and results in procustes (i.e. using it in reconstruct_instances works shockingly well).

Happy to merge this whenever tests pass! Also if you had a quick test case you could add which demonstrated the prior bad behavior of "undesired shifts in the transformed points" that would be great.

[tillns]

Hi Michael
Sorry, I only have limited time for this. I checked the test, and the reason it fails is because it assumes that the transformation matrix should be different with procrustes when non-uniform weights should be used, even if no translation and scaling is applied, only rotation. The way I implemented it now, the weights are not incorporated for the rotation. As I wrote, this would require an iterative optimization instead of an analytical LinAlg algorithm, so I'd say it's beyond the scope of this method. They way I integrated the weights now is that they're used to find the optimal translation and scaling, but they're not used in the computation for the rotation, which is why the test now fails; I had used them there before, but that implementation was faulty. I updated the documentation of the method to explain this. What still works analytically correctly is to use binary weighting, so the user can filter out certain points if they think they're not important with a weights parameter like this [0, 0, 1, 0, 1, 1, ...]. If you agree that this is desired behavior, we could adjust the test to only expect different transformation if translation and/or scaling are enabled. If you'd rather we stick to uniform or binary weights and just forbid other weights completely, I could also adjust the code accordingly.

Examples of bad behavior before:

In the image above, I loaded the blender monkey mesh (left) and created a shifted copy of it (right). If I try to realign the shifted copy with the original using uniform weighting, I get perfect alignment (as expected). However, if I now try to realign the shifted copy with the original using binary weights [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, ....] (so only the first twenty points are weighted 1, all other weighted zero; 1-weighted points are highlighted in red), I get a weird transformation, as shown below:

Note that even the scaling is wrong here. This shows that the previous handling of weights was indeed wrong.

[mikedh]

Sorry for the delay on this! Yeah boolean weights with a todo make sense to me, I'll play around in a branch. Thanks for the PR!