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

Name-ordered circuit parameters #5759

Merged
merged 54 commits into from
Mar 2, 2021

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Feb 1, 2021

Summary

Return the circuit parameters sorted by name and allow binding parameters by list of values. Closes #5557 and #5614.

Details and comments

Details are discussed in https://gist.github.com/Cryoris/1bc894e684f381e07a131196bf86a4da.

In a nutshell:

  • Circuit parameters are sorted by name. Within parameter-vectors, the value indexing has precedence over the name of the index (i.e. we sort x[0] x[1] x[2] ... x[10] and not x[0] x[1] x[10] x[2] ...)
  • The ParameterView object acts as intermediary object between the current return type, which is a set, and a list-like new return type. It implements all set methods but raises a deprecation warning if any is used.
  • The {assign, bind}_parameters methods accept iterables to bind all parameters at once, additionally to the {param: value} dictionaries.

In future we can remove the ordered_parameters property of the n-local circuits and let the variational algorithms use the inherent order of circuits instead of explicitly sorting them by name.

@Cryoris Cryoris requested a review from a team as a code owner February 1, 2021 14:23
@Cryoris Cryoris changed the title [WIP] Sorted circuit parameters Insertion-ordered circuit parameters Feb 1, 2021
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.

Haven't had a chance to review the set methods in depth, but looks good so far.

test/python/circuit/test_parameters.py Outdated Show resolved Hide resolved
test/python/circuit/test_parameters.py Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/parametertable.py Outdated Show resolved Hide resolved
@Cryoris Cryoris requested a review from kdk February 2, 2021 19:29
dest._update_parameter_table(instr)
if front:
# rebuild parameter table
dest._parameter_table.clear()
Copy link
Contributor

Choose a reason for hiding this comment

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

Are parameter tables usually small enough that rebuilding won't be too expensive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tables themselves should be reasonably small, maybe a few hundred elements at max right now. But you've got a point, it would be better to avoid iterating over all gates again (including the unparameterized ones) so I changed the logic to make it more efficient and only work on the parameter tables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it seems we have to rebuild it since instructions are copied and we cannot use the parameter table from the input other. Maybe @kdk has another idea to make it without rebuild 😄

Copy link
Member

Choose a reason for hiding this comment

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

We do something similar in QuantumCircuit.copy:

https://github.com/Qiskit/qiskit-terra/blob/cb2110957e4b22df8d1bfe42e353fae83af2a14b/qiskit/circuit/quantumcircuit.py#L1686

We make the instruction copies ahead of time so that we can build a map from the old to new instances. Then we can use that map to rebuild the parameter table without having to scan and isinstance all the parameters in the circuit. Maybe a similar approach could be useful here.

qiskit/circuit/library/n_local/n_local.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/parametervector.py Outdated Show resolved Hide resolved
qiskit/circuit/parametervector.py Outdated Show resolved Hide resolved
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.

Make it easier to assign parameters to vector of parameters
4 participants