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

Make controller_wrapper functions functors #945

Merged
merged 10 commits into from Sep 24, 2020
Merged

Make controller_wrapper functions functors #945

merged 10 commits into from Sep 24, 2020

Conversation

ghost
Copy link

@ghost ghost commented Sep 21, 2020

Summary

This adjusts the controller_execute functions to be functors rather than free-functions. This allows pickling of these objects - specifically it allows them to be used in multiprocessing.map(...) calls.

Details and comments

See pybind/pybind11#1261

@ghost ghost requested review from atilag, chriseclectic and vvilpas as code owners September 21, 2020 12:28
@chriseclectic chriseclectic added this to the Aer 0.7 milestone Sep 22, 2020
@chriseclectic chriseclectic added the Changelog: Bugfix Include in the Fixed section of the changelog label Sep 22, 2020
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Can you add a test case for the pickle/deepcopy of the backend classes? Something like: https://github.com/Qiskit/retworkx/blob/master/tests/test_deepcopy.py (which is what I added when I had to fix a deepcopy/pickle issue in Qiskit/rustworkx#15)

@ghost
Copy link
Author

ghost commented Sep 22, 2020

Can you add a test case for the pickle/deepcopy of the backend classes? Something like: https://github.com/Qiskit/retworkx/blob/master/tests/test_deepcopy.py (which is what I added when I had to fix a deepcopy/pickle issue in Qiskit/retworkx#15)

I didn't know copy.deepcopy used the functionality I think we're going for .. but it kind of makes sense, cool.

I might've gone a bit overboard but lmk what you think.

Copy link
Contributor

@vvilpas vvilpas left a comment

Choose a reason for hiding this comment

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

Looks good!

@chriseclectic chriseclectic merged commit 3c5a6a2 into Qiskit:master Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the Fixed section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants