[SciPy-Dev] Feedback to students applying to Rotation Formalism in 3D GSoC project

Sat Mar 24 18:36:13 EDT 2018

I decided to give feedback to all the students in one letter. My feedback is proportional to the level of thoroughness of a proposal and I focus on the weakest points. There are strong points as well and we value them, however I don't mention them here (sorry).

To Vishal Gupta https://docs.google.com/document/d/1y5OalGAvYkk8UvLtFjQ2-xhRj_NTwq0fV3GvlCBUp0I/edit

1. Quaternion section description is weird. What's this about "Method would accept a 3x1 Axis unit vector and theta which will be used to generate a 4x1 Quaternion (real, i, j ,k). Also add a method to return a 3x3 Rotation Matrix."?

2. I think it's better to dedicate a separate API section rather than putting it into milestones.

3. When you put API section, try to make it more detailed. Discuss fine points and conventions.

4. Add references.

To Aditya Bharti https://docs.google.com/document/d/1UriyLABwgjUcYfBofSr4hqAnLzblJ71pkzm7i26O8ws/edit

1. I think that returning Euler angles corresponding to the fixed sequence is not satisfactory. So researching on how to implement that is valuable. I personally don't know much about working with extrinsic rotations, from my experience they are not used that often. Implementing conversion to arbitrary intrinsic rotations (A-B-A or A-B-C) is certainly doable, but challenging. 

2. What other important operation similar to composition (in some sense) you can think of.

3. apply method is not discussed enough. Leave the details to figure out to you.

4. slerp, random_sample and Wahba are all inconsiderate. Give a real thought about these algorithms and their APIs.

5. Quaternion class is also inconsiderate. Give a real thought or explanation why you believe is necessary. For example, if you think that quaternion is a superior representation why not to use it in Rotation.

6. I think that you can at least mention other possible algorithms (like if "I have time"). I still believe that "Quaternion spline" is a great algorithm, which will be good to introduce.

To Samyak Jain https://docs.google.com/document/d/12cP0LTFa0H3aQqJfb1WAmr5A_KU6bN3qZ17LbKd5YzM/edit

1. It's good that you honestly mentioned your planned vacation, however it certainly doesn't add you any points. I don't know what to advice you here.

2. It's good that you added theoretical descriptions, but they are not quite right at all points. Probably not the most important thing at the moment.

3. API design seems inconsistent and strange. You first mention Rotation class, but then introduce standalone function (looks like) which return 4-tuple "quaternions". It doesn't look very well thought-out.

4. References to MATLAB are not evil by itself, but you should explain what you want from them. It is not an available implementation you can use as a code source.

To Anubhav Patel https://docs.google.com/document/d/1ylzugkvVYI7m3IXsWVLD4EkE6zQd8B9YDPKtTno9f74/edit

1. Fixing the sequence of Euler angles is TOO restrictive, we can't do that. It is applicable to both "from" and "to" conversions.

2. I don't necessary see the benefits of having two possible ways to initialize the class: from __init__ and from "factory methods". Can we do it better? It's not a critical point for sure.

3. In setfrommatrix --- there is a reference listed in your proposal, where a robust and insensitive to non-orthogonality algorithm is proposed, can you spot it and suggest for this?

4. See hint 2 for Aditya Bharti.

5. I feel that rotate discussion is not detailed enough. What exactly Rotation.rotate(v) does? Is there only one way to apply "rotation"?

6. APIs for slerp, qspline and davenportq are not great. Think about how to improve them.

7. In your Timeline: you goes too much in internals of implementing qspline. It is good in one sense, however you should use understandable descriptions, not "_rates", "_slew", etc.. You are not rewriting the code, you implementing the algorithm from scratch.

To Sudheer Achary https://docs.google.com/document/d/13U4feVYTJJfGFgSHA37p_JxpexBKZJ2-VSjs4z7owcQ/edit

1. Why __init__ accepts Euler angles?

2. What is the reasoning behind introducing rotate_... and other similar methods? These are some static methods. Wouldn't it be more logical that Rotation itself rotates vectors?

3. I don't see a way to compose rotations.

4. I don't how to put it politely, but overall I don't get your API design. It makes some sense, but it just seems really inconvenient and verbose and hard to grasp. What didn't you like about Rotation being some abstraction that can rotate vectors, be composed with other Rotation and converted from/to common representations?

5. Maybe it would be better if you write method signatures appropriately and not just ().

--------------------------

OK, that's it. All proposals and my comments are opened for everybody, so you can learn from them freely. Apologies if I wasn't very attentive to some details, there are many proposals.

If any of the students want to ask or say something, I suggest to use this mail thread.

Best regards,

Nikolay Mayorov

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20180325/84659d88/attachment-0001.html>