API review

Proposer: John Hsu

Present at review:

  • List reviewers

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

Tully

  • It looks like there are three distinct areas at the moment. Simple math functions like clamp and the angle calculations. One statistical function. And the math expression evaluation. I feel as though they should probably be separate.
  • If we want to do the statisitical we should have more than one, and a standalone function w/o the algorithm to update it is a little odd. (aka exponential smooting update step only)
  • Are there no other sources of Math expression evaluation libraries?

Ioan

  • I agree that the math_utils should probably be separated (maybe some smaller subfolders? (algebra, satistics, math_eval)). For statistics however, I suspect more complete code can be found online.
  • I spent some time trying to find a math expression evaluator, and I found only one (forgot the name). However, it had some small bugs, missing features, and the license did not allow free commercial use. I then decided to write one from scratch.

John

  • I think I'd prefer keeping math_utils to a single level directory (no subfolders) given that the original intent of math_utils package is a collection of very simple and nearly trivial mathematical functions that can become an annoyance if repeated too many times.
  • Suggestion:
    • Basic filters (e.g. saturation, running average, etc). Velocity smoothing maybe a candidate for filter functions.
    • Unit conversions should be here.
    • Analytical matrix inversions for small matrices (2X2 and 3X3?)
    • Rotation matrices

Josh

  • I don't think any matrix/transform stuff should go in here. That should be standardized as whatever TF is using (currently Bullet).

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

  • Ken: anything named foo_utils is a bad idea, because it becomes a dumping ground. Should split into multiple packages and get rid of math_utils.
  • /!\ Remove math_utils.

  • /!\ Create new package angles, with angular math in it, which includes radians<->degrees.

    • Update all users
  • /!\ Remove min and max, if they exist; use std::min and std::max instead

  • /!\ Move clamp and exponential_smoothing into controls_toolbox, in filters.h (this might change later)

    • Update all users
  • /!\ Create new package math_expr, with evaluation functions.

    • Remove variants that take char* instead of std::string&

    • Hide ContainsOperators() method

    • Update all users
  • Result: conditional pass, in that math_utils will die, and we agree on the APIs for the two new packages.


Wiki: angles/Reviews/2008-11-07_API_Review (last edited 2009-08-14 20:54:26 by localhost)