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

Copying Nengo objects #977

Closed
4 tasks done
tbekolay opened this issue Mar 8, 2016 · 11 comments
Closed
4 tasks done

Copying Nengo objects #977

tbekolay opened this issue Mar 8, 2016 · 11 comments

Comments

@tbekolay
Copy link
Member

tbekolay commented Mar 8, 2016

From today's meeting, it would be nice to be able to copy Nengo object's cleanly. I think in general we want shallow copies (i.e., don't deepcopy the parameter values); deepcopies aren't necessary since everything we do to make sure we can make multiple simulators also means we don't need to do deepcopies.

Todo:

  • Implement __getstate__ __setstate__ (see pickle-config branch; ensure we don't duplicate code with FrozenObject which does some copy stuff)
  • Implement .copy method
    • Include an add_to_network arg on the copy method that adds or doesn't add to current Network.context
  • Make sure it plays well with Python's copy module
@tbekolay tbekolay added this to the 2.1.1 release milestone Mar 8, 2016
@tbekolay
Copy link
Member Author

tbekolay commented Mar 8, 2016

Also, related to #941.

@jgosmann jgosmann self-assigned this Mar 15, 2016
@jgosmann
Copy link
Collaborator

I started implementing this and it seems to be possible in a quite general manner. The added copy method on NengoObjects can take an add_to_network argument, but neither the Python copy.copy function can nor the unpickling functions. Should we add the object in those cases to the network or shouldn't we? Or maybe only if we are in the context of a network?

Probably we don't want to add the object always to a network (otherwise unpickling outside of a network would not be possible because it would raise an exception).

@hunse
Copy link
Collaborator

hunse commented Mar 16, 2016

Yeah, I would say don't add it when unpickling.

Maybe we shouldn't allow copy.copy at all? We could just have a NotImplementedError that tells people to use our copy method. That would make things more straightforward, and I don't think it really makes anything inconvenient, either.

@jgosmann jgosmann mentioned this issue Mar 16, 2016
3 tasks
@jgosmann
Copy link
Collaborator

I'm using __getstate__ and __setstate__. I have to, to support pickling/unpickling and it gives me copying basically for free. But because of that it is not possible to raise NotImplementedError for the Python copy function. Furthermore, I'm using exactly that function in our copy implementation.

@hunse
Copy link
Collaborator

hunse commented Mar 16, 2016

What about something like this?

@jgosmann
Copy link
Collaborator

Isn't this using a private method in the copy module?

@hunse
Copy link
Collaborator

hunse commented Mar 16, 2016

Yep! Probably a bit strange, but I basically just took out the parts of their code that do copying if there's no __copy__ function. Though we should check if this works in 3.x. Also, maybe better to just write all our own code to do it using __getstate__ and __setstate__, which shouldn't be too hard. I just threw it together quick as a proof of concept.

@jgosmann
Copy link
Collaborator

To me it seems to be slightly more effort to disable Python's copy than it is worth. It's actually fully functional, the copy just doesn't get added to the network which might actually not to surprising (and of course in examples we would prefer NengoObject.copy where there is an add_to_network argument).

@hunse
Copy link
Collaborator

hunse commented Mar 16, 2016

Yeah, I just don't want people to be confused or surprised by it not getting added to the model. I feel like on the user side, we want the default behaviour to be to add it to the network, since I think that's what users will expect. It seems strange if our copy does it one way and Python's copy does it the other.

@jgosmann
Copy link
Collaborator

I finished the PR. Using Python's copy or unpickling in the context of a network will produce a warning that the object was not added to the network. This still allows to use Pythonscopy` but the warning should make it clear that this might not have the intended effect.

@jgosmann jgosmann removed their assignment Mar 17, 2016
@jgosmann jgosmann mentioned this issue Mar 24, 2016
@tbekolay tbekolay modified the milestones: 2.2.0 release, 2.1.1 release Jun 11, 2016
@tbekolay tbekolay removed this from the 2.2.0 release milestone Aug 8, 2016
@drasmuss
Copy link
Member

Copying of Nengo objects is implemented now.

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

No branches or pull requests

4 participants