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

Upgrading bind_parameters: allow in place and mixed floats + parameters #4037

Merged
merged 29 commits into from
Apr 8, 2020

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Mar 29, 2020

Summary

In the current situation we need to use bind_parameters to bind float values to a circuit (which returns a copy of the original) and _substitute_parameters to replace the existing parameters with new Parameter objects (which modifies the circuit in place).

With this PR the bind_parameter method gets an upgrade such that it can bind both numeric values and Parameter objects and can do so in-place or on a copy.
The signature is (type hints not actually included, just for explaining reasons here)

def bind_parameters(value_dict: Dict[Parameter: Union[Number, Parameter]], in_place: bool = False)

This is much needed functionality for circuits to become native objects in Aqua, where parameters are replaced a lot and it is cumbersome to always have to distinct between numeric values and Parameters.

Also a num_parameters attribute is added to the circuit for the same integration reason.

Details and comments

This allows to do things such as

>>> theta, x, y = Parameter('θ'), Parameter('x'), Parameter('y')
>>> qc = QuantumCircuit(1)
>>> qc.rx(theta, 0)
>>> qc.u3(0, theta, x, 0)
>>> qc.bind_parameters({theta: 2, x: y}, in_place=True)
>>> print(qc.draw())
        ┌───────┐┌───────────┐
q_0: |0>┤ Rx(2) ├┤ U3(0,2,y) ├
        └───────┘└───────────┘

@Cryoris Cryoris added this to the 0.13 milestone Mar 29, 2020
@kdk kdk added the Changelog: API Change Include in the "Changed" section of the changelog label Mar 30, 2020
Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

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

This looks good, few comments in line. I wonder if it would make sense to rename the new method to sub_parameters or similar since binding implies a degree of finality. i.e. once bound, a circuit is ready for execution; you can only bind once; you can't pass a Parameter to execute(parameter_binds=...);...

qiskit/circuit/equivalence.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
@Cryoris
Copy link
Contributor Author

Cryoris commented Mar 30, 2020

Concerning the naming we could also use set_parameters, maybe this is the most generic. This also somewhat suggests that the behaviour is not equal to the old bind or substitute, which are still used in the parameter expression.

qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
test/python/circuit/test_parameters.py Show resolved Hide resolved
qiskit/circuit/equivalence.py Show resolved Hide resolved
Copy link
Member

@kdk kdk 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, two small comments and then this is good to go.

test/python/circuit/test_parameters.py Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
@Cryoris Cryoris requested a review from kdk April 8, 2020 21:03
@kdk kdk added the automerge label Apr 8, 2020
@mergify mergify bot merged commit 1abc489 into Qiskit:master Apr 8, 2020
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
…rs (Qiskit#4037)

* add num parameters keyword

* upgrade bind_parameters: in_place and mixed params

* can now bind/substitute using the in_place kwarg
* can bind floats and Params in the same dict

* add comments, better variable naming

* add reno, use bind_params instead of _subst_params

* add test for num_parameters property

* apply suggested changes

mainly rename in_place to inplace

* rename bind to assign_parameters

still keeping bind_parameters in some places in the test

* Update releasenotes/notes/bind-parameters-mixed-values-and-in-place-48a68c882a03070b.yaml

Co-Authored-By: Kevin Krsulich <[email protected]>

* Update qiskit/circuit/quantumcircuit.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* Update qiskit/circuit/quantumcircuit.py

Co-Authored-By: Kevin Krsulich <[email protected]>

* remove _subst_param from reno

* delete _partition_dict

* throw error upon ParameterExpr in bind_parameters

* revert changes on bind_, test both assign+bind

* assertions must be in the loop

* complete the docstring

Co-authored-by: Kevin Krsulich <[email protected]>
Co-authored-by: Kevin Krsulich <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants