Skip to content
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

Override __getstate__ and __setstate__ #12

Closed
wants to merge 2 commits into from

Conversation

rizar
Copy link
Contributor

@rizar rizar commented Oct 24, 2014

We should decide to which extent the bricks should be copyable.
I think that it should be possible to clone a hierarchy of bricks
but without copying the parameters. The reason behind is that
copy.[deep]copy and SharedVariable.copy behave very differently.
The first actually copies the content of the variable, whereas
the second simple creates another Theano variable. To prevent the
possible confusions it is easier to avoid implicit copying of
parameters.

We should decide to which extent the bricks should be copyable.
I think that it should be possible to clone a hierarchy of bricks
but without copying the parameters. The reason behind is that
`copy.[deep]copy` and `SharedVariable.copy` behave very differently.
The first actually copies the content of the variable, whereas
the second simple creates another Theano variable. To prevent the
possible confusions it is easier to avoid implicit copying of
parameters.
Ensures that `params` are not copied even when `copy.deepcopy`
is called by excluding them from the brick's state together
with the attributes `allocated` and `initialized`.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Docstrings are supposed to end with a blank line in the NumPy docstring standard.

@bartvm
Copy link
Member

bartvm commented Oct 25, 2014

I'm not sure about overriding deepcopy's behaviour. It seems counterintuitive to not make deep copies of all the configuration/parameters in this case. I feel that there are two different ways in which we want to copy a brick:

  • We want to use the same brick in two places. This means that these two brick are really one and the same, except that their name is different perhaps. We could do this using something like:
class BrickCopy(object):
    def __init__(self, brick, name):
        self.brick = brick
        self.name = name
    def __getattribute__(self, attr):
        if attr == 'name':
            return self.name
        else:
            return getattr(self.brick, attr)

class Brick(object):
    def copy(self, 'name'):
        return BrickCopy(self, name)
  • We want to configure a complicated brick once, and then make a few copies for which we can change the settings a little bit. This would be kind of like deepcopy's standard behaviour. In this case we want to have deep copies of all the parameters as well (i.e. the copies don't share parameters anymore).

@rizar
Copy link
Contributor Author

rizar commented Oct 25, 2014

I am more interested in the second way of copying, instead of the first one I would just use a brick two times.

You are right that sometimes copying the parameters together with a brick makes sense, e.g. when these are loaded parameters. But when they were randomly initialized, we have a confusing situation when two bricks have completely similar initializations, though the configuration says they should be random.

Another tricky case: what if parameters were replaced, like in case of an autoencoder with shared weights?

@rizar rizar closed this Nov 14, 2014
@rizar rizar deleted the cloning branch February 23, 2015 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants