Change Parameter equality from python id() to internal uuid(). #2947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kdk
requested review from
1ucian0,
ajavadia,
atilag,
chriseclectic,
delapuente,
diego-plan9,
ewinston,
jaygambetta,
lcapelluto,
mtreinish,
nonhermitian and
taalexander
as code owners
kdk
force-pushed
the
pickleable-parameters
branch
from
cd767e4
to
1d45bb2
Compare
mtreinish
approved these changes
Aug 14, 2019
|
Azure is acting up once again. The test jobs all succeeded here: https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=1852 but the github <-> azure communication fell apart and the checks list didn't update for 2 of the jobs. Since the tests all passed I'm merging this now. |
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
…t#2947) Parameters were defined such that they would only ever __eq__ self. That way, if a user defined a Parameter('theta') and wanted to compose in a subcircuit defined elsewhere which contained a different Parameter('theta'), we would be able to detect that the parameter names overlapped and raise an error. However, if a user sent a Parameter to a different python instance though (say through multiprocessing.Pool or qiskit.tools.parallel_map), it would be instantiated as a different python object and so no longer be considered equal to the original parameter. This PR changes Parameter to instead generate a random UUID on instantiation and use that for equality testing. Unlike id(self), self._uuid will be preserved across pickling and de-pickling, but should otherwise preserve Parameter's equality behavior. Fixes Qiskit#2429 Fixes Qiskit#2864
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2429
Fixes #2864
Details and comments
Parameters were defined such that they would only ever__eq__ self. That way, if a user defined aParameter('theta')and wanted to compose in a subcircuit defined elsewhere which contained a differentParameter('theta'), we would be able to detect that the parameter names overlapped and raise an error.However, if a user sent a
Parameterto a different python instance though (say throughmultiprocessing.Poolorqiskit.tools.parallel_map), it would be instantiated as a different python object and so no longer be considered equal to the original parameter.This PR changes
Parameterto instead generate a random UUID on instantiation and use that for equality testing. Unlikeid(self),self._uuidwill be preserved across pickling and de-pickling, but should otherwise preserveParameter's equality behavior.