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

Pickleable signals #1135

Merged
merged 1 commit into from
Dec 29, 2017
Merged

Pickleable signals #1135

merged 1 commit into from
Dec 29, 2017

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Jul 29, 2016

Description:

This PR makes Signals pickle properly.

Motivation and context:

With the copy branch (#984), we can now start thinking about pickling whole Models. I tried this out, but the simulator steps were getting generated in random orders after reloading the Model. I tracked this down to the fact that when we pickle signals, specifically those that are views on other signals, the pickling process (I was using the dill package, since it is more flexible) didn't maintain views between the initial_value arrays of signals. Since we rely on this in the Signal.may_share_memory function, that was failing.

Interactions with other PRs:

This depends on #984.

How has this been tested?

Added a test which pickles a Model object and then unpickles it, ensuring that the simulation with both the original and re-created objects are the same.

Where should a reviewer start?

It's quite short.

How long should this take to review?

  • Quick (less than 40 lines changed or changes are straightforward)

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • [na] I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@hunse
Copy link
Collaborator Author

hunse commented Nov 16, 2017

This is ready for review.

@nengo nengo deleted a comment from mention-bot Nov 16, 2017
@hunse hunse mentioned this pull request Nov 23, 2017
5 tasks
@tbekolay tbekolay merged commit 93c7b61 into master Dec 29, 2017
@tbekolay tbekolay deleted the pickleable-signals branch December 29, 2017 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants