[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [IMP-dev] too many rotations in IMP

We already have constructing a Rotation3D from your choice of Euler angles (and you can get a matrix from that if you want). So there isn't much reason to be passing them around at all.

No, that is not true. Given that no convention was established, there is freedom to use anyone.
Yes, there is no convention for which types of Euler Angles to use. There is a convention that all rotations be represented using a Rotation3D (which can be created from many things).

Sure, the question is more whether it should be part of the API or in an internal header or .cpp file where it is used. So far I don't see anything that justifies creating an alternative rotation passing convention (which would require lots of functions to be eventually duplicated to support both)._______________________________________________

I will be happy hearing about any improvement in the API. I'm worried though about project(). It should work at least as it is working now, and respecting my choice of angles and rotation interface requesting a matrix, because that function is at the core of my project and I want that performance. If it can be improved, I gladly will acknowledge any contribution. As long as the function is able to access what requires, I don't mind the way it is done. I can provide stringent tests that I decided not to finally put in actual tests.
Given a relatively narrow interface, performance is generally pretty easy to achieve given a benchmark to test changes. The narrow interface is important since it means that you don't have to improve up too much code (eg having several Rotation classes means improvements to one are not available to functions which use another).
I agree.

As I mentioned, we can always add an internal cache of the matrix values in Rotation3D if we find that such speeds things up. My suspicion is that the compiler would cache the results for us if you are applying the rotations in an inner loop and that the memory accesses would overwhelm the reduced number of multiplications if it is not, but it is easy to test once a benchmark is set up. For this, a benchmark of just rotating a matrix of random numbers would probably do and be very easy to set up.
I will be very happy to hear from that.

It seems to me the old interface with
- Rotation3D which has a rotate function
- functions to create Rotation3Ds from various Euler angles (including ZYZ, if it is not already there)
- possible caching of the rotation matrix in Rotation3D if that does speed things up
covers everything that is needed without the addition of any of the new classes and conflicting conventions. Is this right?

No. I tried building a Rotation 3D and using it and it didn't work with project(). I can provide the details of what was failing, and maybe somebody can find what I couldn't after fighting for some weeks. It would be very helpful.

Then anyone can, upon reading Euler angle values (of whatever convention) create a Rotation3D. This Rotation3D can be used to perform rotations in an efficient manner. And we don't have to worry about some functions take one representation for rotations, and some another.

Fine, as long as project() can be accessed the way I want/need and works after the changes, I don't mind the internal way it calls rotations.