theochem / procrustes

Python library for finding the optimal transformation(s) that makes two matrices as close as possible to each other.
https://procrustes.qcdevs.org/
GNU General Public License v3.0
109 stars 20 forks source link

Would be nice to have the used scaling and translation returned in the answer #166

Closed esoinila closed 2 years ago

esoinila commented 2 years ago

Now the scaling factor and translation vector are calculated in the utilities but they are not returned in the answer (result = orthogonal(a, b, scale=True, translate=True)). For 3D-graphics calculations, these would be handy to scale and translate any new points found from the distance matrix (I get preliminary coordinates from MDS) back into the real coordinates of b.

FanwangM commented 2 years ago

Thank you for your interest in our work!

To get the scaling factor, you can call the function _scale_array after importing,

from procrustes.utils import _scale_array

And the scaling factor can be accessed as the second element of the returned tuple. More details https://github.com/theochem/procrustes/blob/8454b837fbf0ce1cf695b736b5e145108849ef32/procrustes/utils.py#L158-184.

For translation matrix, it's almost the same.

from procrustes.utils import _translate_array

For changing the return to include these two elements, is this a often case in your field? We may discuss within our team and then get it back to you. @esoinila

esoinila commented 2 years ago

Thanks for the quick reply. I will test those methods on Monday.

My use case is the case where I want to add new datapoints into 3D word without changing the original 3D world coordinates. Suppose in-door navigation with something like bluetooth signal strength giving approximate distances to devices as distance matrix. For some beacons we know their positions beforehand (we have placed them into indoor-space intentionally into known locations as beacons). Ideally we want to use these known positions to calculate a transition for putting the new distance matrix point(s) on the 3D-map into correct position(s) (a new point could be your phone with bluetooth on).

Instead of comparing points from distance matrix to known point positions in arbitrary scaled/translated coordinates on the axes we could have correct 3D-world coordinates (from matrix b) on the axes (see image).

One way to accomplish this robustly would be to get used scale and translation so these changes can be reversed (this is my current approach). Even fancier option would be to have an option to use b-matrix coordinates in results.

image

The map-use-case in Ivan Dokmanics et alls article http://dokmanic.ece.illinois.edu/assets/pdf/Dokmanic2015eg.pdf is quite close to my use-case (FIGS1 box on page 13) except that it is in 2D.

PaulWAyers commented 2 years ago

@fwmeng88 and @FarnazH: we should think about whether we want to return the scale/translate or support it only as @fwmeng88 mentions. It is a substantial revision (albeit an easy one) to adjust all the returns.

I don't have a strong feeling for pros vs. cons. If we were doing it all over again, I guess that returning the translation/scaling automatically might have been sensible. The con I see is only that it makes the return a lot more complicated, and the scaling/translation is not always done (it's an option) and so we want to return None that frequently?

It may be that a documentation fix is more appropriate, then. Y'all's thoughts? We might at least not make these methods "hidden/internal" methods.

esoinila commented 2 years ago

@fwmeng88 Thanks, The use of: from procrustes.utils import _scale_array from procrustes.utils import _translate_array scaled_a, scale = _scale_array(a,b) translated_a, reverse_vector = _translate_array(a,b) seems to work in jupyter notebook, even though those methods start with underscore.

FanwangM commented 2 years ago

No problem at all.

Yes, these are private functions, but it's great that it's useful for you. Please let me know if you have further questions. @esoinila

FanwangM commented 2 years ago

I thought about this again and maybe we don't need to have a return of scaling factor or translation matrix as in most cases, this makes too many returns. I would favor clear documentation, somewhere like https://procrustes.qcdevs.org/notebooks/Quick_Start.html? @PaulWAyers @FarnazH

PaulWAyers commented 2 years ago

Can you update the documentation and close this issue @FanwangM ?

PaulWAyers commented 1 year ago

Just to be clear, this is in the quickstart.ipynb Jupyter notebook.

@FanwangM I made a few tweaks to verbiage, which are hopefully OK.

FanwangM commented 1 year ago

Thank you. I will take a look soon. @PaulWAyers