New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copyable - fast robust copy() throughout Menpo. #398

Merged
merged 8 commits into from Jul 18, 2014

Conversation

Projects
None yet
3 participants
@jabooth
Member

jabooth commented Jul 18, 2014

Background

When we started Menpo, deepcopy() was our weapon of choice whenever we had to copy anything. We didn't think too long and hard about how deepcopy() worked, but it worked well, reliably copying any object graph without fuss.

We then discovered a problem though - deepcopy() was really slow on smaller objects like Homogeneous transforms, to the point where it was slowing down algorithms significantly (remember, every single .apply(), .compose_before() and .from_vector() causes a copy to be made).

To get around this, we decided follow suit with numpy and add a dedicated .copy() method to our types. This method was all about speed - it didn't have to guarantee copying ducked-typed instances on classes, it just had to be really fast.

We went to task adding class specific .copy() methods everywhere, and found that calling them was much faster than deepcopy(). Most of these methods rebuilt a fresh instance of type(self) using the constructor, commonly making use of the copy=True flags that we have added over time. Here's PointCloud.copy() for example:

def copy(self):  # PointCloud
    new_pc = PointCloud(self.points, copy=True)
    new_pc.landmarks = self.landmarks
    return new_pc

The problem

This worked well, but it adds an interface that needs to be maintained all over the code. Ideally, all types would have a .copy() method so that we can call .copy() with confidence anywhere and get a fast copy that works. That's a lot of methods to write and maintain whenever a change is made to the code.

A brief guide to Python object creation

Back when I wrote the Homogeneous.copy() method I had a problem. Each of the Homogeneous transforms had a very different constructor, so I was going to have to write lots of copy methods which seemed like a waste - after all, I know that each homogeneous only has a single piece of state on it - namely the homogeneous matrix.

In digging into this problem I learnt a lot more about Python object creation. Specifically, when you build a new object two calls are made by Python for you:

foo = Foo(*args, **kwargs)

is the same as:

foo = Foo.__new__(Foo, *args, **kwargs)
Foo.__init__(foo, *args, **kwargs)

The __new__ static method is the one that actually gives you back an object of type Foo. That is to say, if we were to stop half way through the usual initialisation and inspect foo we would have an object with all it's methods bound. The __init__ as we well know is where we can then actually set up the object's state. (If you think about it, you already knew that __new__ must return you an actual instance of the object, that's why in __init__ we are able to call methods on self - self has already been __new__'d up for us, we just never had to think about it before).

Hint at the solution - it's in master already

Knowing this, I wrote a single .copy() method on Homogeneous that worked for every subclass:

def copy(self):  # Homogeneous
    new = self.__class__.__new__(self.__class__)
    # new is now a valid type(self), but __init__ has not been called!
    # That's OK, I know that *this* transform passed the init OK so
    # no need to do the work again - just set the _h_matrix and we
    # are done
    new._h_matrix = self._h_matrix.copy()
    return new

This has been in master for months and we have all been happily relying on it working, and it works great. It's also pretty much as fast as we can get a copy - there is no validation code recalled in the __init__, we just pay for Python object initialisation and a single numpy .copy().

A brief guide to Python object state

For the last few days I've had the nagging sense that we are missing a trick, and maybe we can make this __new__ + manually set copied state more general to save us writing specific copy methods. Turns out it's actually really straight forward, be we need to understand a little about object instance state first.

When you assign to an object instance, Python actually stores the reference to what you want in a dictionary on each instance called __dict__.

foo = Foo()
foo.bar = 2

is actually doing

foo = Foo()
foo.__dict__['bar'] = 2

You can easily verify this for yourself - just take a look at __dict__ on any object to see all the state that object owns.

Building a general solution

Let's get started on what we want to implement. In typical Menpo style we will have a Copyable interface that all our types will impliment:

class Copyable(object):

    def copy(self):
        pass  # implement me!

Let's imagine a world where all objects are Copyable and can only hold onto other Copyable objects. Then our generalised copy method is super simple:

class Copyable(object):

    def copy(self):
        new = self.__class__.__new__(self.__class__)
        for k, v in self.__dict__.iteritems():
            new.__dict__[k] = v.copy()
        return new

Note that this is rock solid, and would work for any case with one caveat - you better not have any cyclic references or you will have an infinite loop on your hands. We actually will not address this issue in this PR. Checking for cyclic references is exactly what deepcopy() is doing, and as we know we aren't willing to pay the performance price. Keep this in mind.

Of course this hypothetical situation is ludicrous - some objects will have to hold stuff other than just other Copyable's, otherwise what's the point. Let's extend the interface to deal with what is by far the most common type of state Menpo objects hold - ndarray instances.

class Copyable(object):

    def copy(self):
        new = self.__class__.__new__(self.__class__)
        for k, v in self.__dict__.iteritems():
            new.__dict__[k] = v.copy()  # fine for Copyable or ndarray
        return new

Yup, that solution actually works just fine for ndarrays too. The arrays will actually get copied using the native high performance copy.

So what other kinds of objects do Menpo instances hold on to? Certainly some objects have strings, integers, or sometimes None attached to them. Let's try and make Copyable resilient to them:

class Copyable(object):

    def copy(self):
        new = self.__class__.__new__(self.__class__)
        for k, v in self.__dict__.iteritems():
            try:
                new.__dict__[k] = v.copy()  # fine for Copyable or ndarray
            except AttributeError:  # if .copy() doesn't exist
                new._dict__[k] = v  # assume immutable 
        return new

Note that this actually deals with all the above types, because they are all immutable. That means it's totally fine to hold onto a reference to the original - it can never change, so a copy is actually redundant.

This is actually pretty much the proposed interface for Copyable in Menpo now. Every object has this method on it, and thus can be copied robustly at high speeds - so long as we don't have cyclic references.

But wait! What about other stuff like dicts or lists on classes?

Sure, some Menpo types have other things on them that need special care. For instance a LandmarkManger stores a dict mapping strings to LandmarkGroups. That's fine, in those rare cases we override .copy() to deal with the edge cases:

def copy(self):  # LandmarkManager
    # do a normal copy.
    new = Copyable.copy(self)
    # The dict has been shallow copied - rectify that here
    for k, v in new._landmark_groups.iteritems():
        new._landmark_groups[k] = v.copy()
    return new

Notice how it's generally quite performant to invoke the standard Copyable.copy() as it just won't copy things that don't have .copy() on them, then you can be corrective in the specialisation.

How common do these edge cases crop up?

I've added a little commented-out debug code to be able to answer this question. If you uncomment the code in menpo/base.py every time a copy is made we store out some key stats. Call print_copyable_log() to see what has happened. Here's the output after running all our unit tests:

Has .copy() but not Copyable:
  dict           |  MaskedImage, LandmarkManager, LandmarkGroup
  ndarray        |  CachedPWA, MaskedImage, ColouredTriMesh, MeanInstanceLinearModel, Similarity
               ...  Image, Affine, MockedVInvertable, OrthoMDTransform, BooleanImage, Translation
               ...  OrthoPDM, UniformScale, TriMesh, Homogeneous, Rotation, PointCloud
               ...  NonUniformScale, TexturedTriMesh, PCAModel

No .copy() (shallow copied):
  IOInfo         |  MaskedImage, LandmarkGroup
  int            |  PCAModel
  ImageFeatures  |  MaskedImage, Image, BooleanImage
  list           |  TransformChain, MockedComposable
  bool           |  PCAModel
  CLookupPWA     |  CachedPWA
  str            |  LandmarkGroup
  NoneType       |  MaskedImage, BooleanImage, Image, OrthoMDTransform, TriMesh, PointCloud
               ...  PCAModel

if we filter out the cases which we are happy with (ndarray.copy(), immutable non-copies) we are left with:

Has .copy() but not Copyable:
  dict           |  MaskedImage, LandmarkManager, LandmarkGroup

No .copy() (shallow copied):
  ImageFeatures  |  MaskedImage, Image, BooleanImage
  list           |  TransformChain, MockedComposable
  CLookupPWA     |  CachedPWA

so we currently need specialisation of .copy() on Image, TransformChain, CachedPWA (and indeed any wrapper around an extension object), LandmarkManager and LandmarkGroup. That's it, every other Menpo object works great with the above code with no specialisation.

Comments

  • This PR also includes a few fixes for landmarks arising after #395.
  • Image won't need it's specialised copy() once the .features object is removed.
  • IOInfo is now considered immutable, so it will survive copying just fine.
  • Copyable.copy() will handle duck-typed attributes just fine, so long as they don't cause circular references. Bear in mind things like dictionaries will be shallow copied though.
  • When you want a 100% copy and don't care about speed, deepcopy() will work fine. It's basically Copyable.copy() with many more checks for edge cases. We are trading flexibility for performance here.

TODOs

  • No effort is presently made to deal with all cython types - may need some thought.
  • Decision needs to be taken on what it means to copy a TransformChain - this maintains the status quo (i.e. don't copy each transform). Maybe should change.
  • Not sure why a MaskedImage has a dict on it somewhere in the test suite - need to investigate

@jabooth jabooth changed the title from Menpocopy to Copyable - fast robust copy() throughout Menpo. Jul 18, 2014

@nontas

This comment has been minimized.

Member

nontas commented Jul 18, 2014

It looks as a really good and useful change! So the basic idea is that we avoid the cost of calling __init__ right? What's the speed-up compared to our previous copy() version? Awesome explanation btw!

@jabooth

This comment has been minimized.

Member

jabooth commented Jul 18, 2014

It's true that we do avoid __init__ which is a nice performance bump, but the main win is that we don't have to implement .copy() for the majority of our types, and never have to use deepcopy() which really hurt us before. Performance is up for .copy() methods where we used to go through __init__ on master, which is all of them bar Homogeneous. I'll do a comparison and report back. And thanks mate!

@jalabort

This comment has been minimized.

Member

jalabort commented Jul 18, 2014

This looks very good James. I think I will have to read it a couple of times more to completely understand it but I got the general idea. Nicely explained.

jabooth added some commits Jul 18, 2014

jabooth added a commit that referenced this pull request Jul 18, 2014

Merge pull request #398 from jabooth/menpocopy
Copyable - fast robust copy() throughout Menpo.

@jabooth jabooth merged commit 8f86167 into menpo:master Jul 18, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@jabooth jabooth deleted the jabooth:menpocopy branch Jul 18, 2014

@jabooth jabooth referenced this pull request Aug 26, 2014

Closed

Serialization of data #164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment