Improve the superellipsoid surface area implementationBRL-CAD
Status: ClosedTime to complete: 120 hrs Mentors: SeanTags: c, code cleanup, refactoring, improvement

This task is a follow-on of http://www.google-melange.com/gci/task/view/google/gci2013/5486014072094720

The objective of this task is to improve the surface area implementation by addressing the issues identified in the comments as well as attempting to improve performance.

Submit a patch that improves the existing implementation.

 

Uploaded Work
File name/URLFile sizeDate submitted
superell_surface_area_improvements.tar.gz50.1 KBJanuary 03 2014 13:54 UTC
Comments
Peter Amidonon January 2 2014 06:17 UTCTask Claimed

I would like to work on this task.

Mandeep Kaur on January 2 2014 06:32 UTCTask Assigned

This task has been assigned to Andromeda Galaxy. You have 120 hours to complete this task, good luck!

Peter Amidonon January 3 2014 13:54 UTCReady for review

The work on this task is ready to be reviewed.

Sean on January 3 2014 14:24 UTCTask Closed

Congratulations, this task has been completed successfully.

Peter Amidonon January 3 2014 14:33 UTCPerformance improvement

Sorry, I forgot to post these numbers in the explanation file: 10 runs of the function that start and end at a precision of 4096 took these amounts of time for slow (original) and fast (new) implementations:



FAST: 2m2.971s
SLOW: 5m39.673s

Peter Amidonon January 3 2014 14:35 UTCOne more performance improvement

I had one more idea for another performance improvement after I submitted this task; if you can open another task for that, I can do it as well, or I could do it after Code-In is complete.

Sean on January 3 2014 14:35 UTCsome problems

Andromeda, this looks good but you did introduce a new constant (500000) without documenting it.  Also, I note that your documenting of the other constants (both 64 before and 1024 now, and the multiplied by 4 or 2) are inane... :)  Your explanation for both is the same, which means it was useless for 64 and is probably still useless for 1024.  All you basically said is "this is what seemed to work, I guess" even though they're two orders of magnitude different!


Also the fact that you wrote "A significant speedup of superell_surf_area_general could make larger values more practical." implies these values are completely tied to how fast your hardware is, which they shouldn't be.


The constant values should be picked based on some metric of quality, but NOT overall performance or time.  That is to say, basing it on quality would be reducing computed area error to less than a given value.  Basing it on time would be like adding a timer and refining the estimate until too much time has elapsed, but you're not doing that (nor would that be a great idea).