NonlinearFactor::clone() ??

Issue #13 new
Frank Dellaert created an issue

what is it and why do we need it? Which code in gtsam uses it?

Comments (6)

  1. Alex Cunningham

    Wrap is the most significant use for having clone() because it's a virtual class, from the wrap docs in gtsam.h:

    • Virtual inheritance
      • Specify fully-qualified base classes, i.e. "virtual class Derived : ns::Base {" where "ns" is the namespace
      • Mark with 'virtual' keyword, e.g. "virtual class Base {", and also "virtual class Derived : ns::Base {"
      • Forward declarations must also be marked virtual, e.g. "virtual class ns::Base;" and also "virtual class ns::Derived;"
      • Pure virtual (abstract) classes should list no constructors in this interface file
      • Virtual classes must have a clone() function in C++ (though it does not have to be included in the MATLAB interface). clone() will be called whenever an object copy is needed, instead of using the copy constructor (which is used for non-virtual objects).
      • Signature of clone function - will be called virtually, so must appear at least at the top of the inheritance tree virtual boost::shared_ptr<CLASS_NAME> clone() const;

    I also tend to use it alot because I frequently need to get a factor and generically change all of the keys to slightly different keys. It's more used for the linear factors, though.

  2. Richard Roberts

    Hi Frank, to add one more thing - clone() is a standard-practice way of duplicating a virtual class instance when you have only a base class pointer (see http://en.wikipedia.org/wiki/Cloning_(programming) ). In that sense we use it e.g. to make a deep copy of a factor graph, or to copy base-class objects in the MATLAB wrapper, or in several other places.

    It's needed in MATLAB because the way our wrapper is designed it needs to copy objects referenced by pointer and sometimes has a pointer only to a base class. It might be possible to rework this, but it would eliminate the possibility of creating derived types in C++ that are still handled as their base type in MATLAB, and would be a non-trivial change in the wrapper.

  3. Richard Roberts

    So, my memory isn't 100% on this, but I think the way the memory management in our MATLAB wrapper works is that every MATLAB workspace object is referenced only once - thus when deleting the workspace object we also delete the C++ object. This is true whether the object is referenced in C++ by pointer or by value. So, e.g. if you assign a factor from a graph into a MATLAB variable, the factor is retrieved by pointer, but then the underlying factor object is copied. And likewise if a C++ function returns an object by shared_ptr, we copy the object into a new one to use in MATLAB. This happens even when the pointer is to the base class - e.g. NonlinerFactor* instead of the derived type e.g. BetweenFactor.

    Obviously there's unnecessarying copying taking place, but the reason it ended up like this is we had a lot of difficulties eliminating crashes and memory leaks and this scheme worked. Also, it has seemed that the overhead of this copying is small compared to the built-in overhead of calling C++ functions from MATLAB.

  4. Frank Dellaert reporter

    Hmmm. That was not my design: I remember I expressly set up shared pointers to avoid the copying. I do remember the difficulties with the crashes, though. who were the main developers on this scheme back then?

  5. Log in to comment