ethz-asl / curves

A library of curves for estimation.
BSD 3-Clause "New" or "Revised" License
74 stars 28 forks source link

Better Behaviour Definitions #13

Closed sanderson77 closed 10 years ago

sanderson77 commented 10 years ago

Even for development it may be prudent to better document the expected behaviour of our functions.

e.g. should these function clear the map before adding entries, or simply insert on top of the existing entries (the current case)?

HermiteCoefficientManager:

  /// \brief Get the coefficients that are active within a range \f$[t_s,t_e) \f$.
  void getCoefficientsInRange(Time startTime, 
                              Time endTime, 
                              Coefficient::Map& outCoefficients) const;

  /// \brief Get all of the curve's coefficients.
  void getCoefficients(Coefficient::Map& outCoefficients) const;

LinearInterpolationVectorSpaceCurve:

  /// \brief Get the coefficients that are active at a certain time.
  virtual void getCoefficientsAt(Time time, 
                                 Coefficient::Map& outCoefficients) const;

  /// \brief Get the coefficients that are active within a range \f$[t_s,t_e) \f$.
  virtual void getCoefficientsInRange(Time startTime, 
                                      Time endTime, 
                                      Coefficient::Map& outCoefficients) const;

  /// \brief Get all of the curve's coefficients.
  virtual void getCoefficients(Coefficient::Map& outCoefficients) const;

Is there a case where appending would be useful, and if so, do we ever expect a scenario where we attempt to insert a duplicate key? e.g. this would happen if we called getCoefficientsInRange twice, with two overlapping ranges, and the same outCoefficients map. If we expect duplicates, should we sanity check that the coefficient values are the same?

furgalep commented 10 years ago

These are good questions. Currently I don't know 100% for all questions. I think we will figure out more as we work on the GTSAM interface.

I think these functions should not clear the map before adding entries and I don't think it should be an error for insert a duplicate key.

Let's go with that behaviour and revisit if necessary. @sanderson77 , will you update the comments?

furgalep commented 10 years ago

One more point: I think these functions shouldn't fail if the start and end times fall outside the valid range. They should literally return all coefficients active between those times.

rdube commented 10 years ago

This last issue was fixed in our last commit to the feature/linear_interpolation branch.