qiskit icon indicating copy to clipboard operation
qiskit copied to clipboard

Add rust friendly `assign parameters` methods

Open raynelfss opened this issue 1 year ago • 2 comments

Summary

Follow up on #12794. These commits expose some rust-friendly assign_parameters methods in CircuitData allowing us to perform parameter assignments without having call to python directly.

Details and comments

The following commits add the methods assign_paramters_from_slice and assign_parameters_from_mapping which enable the assignment of parameters in rust in a friendly way instead of using hard-Python structures (Py<_> or Bound<'_, _>. Here are their use cases:

  • assign_paramters_from_slice: Use whenever a collection of Param needs to be assigned. The amount of parameters sent needs to be of the same size as the the number of parameters in the CircuitData instance.
  • assign_parameters_from_mapping: Use whenever of mapping between ParameterUuid instances from the CircuitData and Param instances coming from Gates or Operations. This can be used for partial assignment, and the number of parameters can be smaller or equal to the amount of parameters in the CircuitData instance.

Known issues

  • ~~The mapping of Params : Params may not be optimal as we wouldn't know if the types of parameters we're binding are valid, Param::Float does not posses a uuid and therefore is not a valid key for the mapping.~~ Will do a mapping between ParameterUuid and Param to assume that all keys are valid instances with valid uuids.
  • ~~The mapping of assign_parameters_from_mapping does not accept instances of Map<Param, Param> due to param not implementing the hash property in Rust.~~ Now it accepts any mapping of ParameterUuid : Param as long as it can be turned into an iterator of type (ParameterUuid, Param).

Blockers

  • [x] #12926

Other comments

  • Feel free to point out any issues in the comments and I will quickly address.

raynelfss avatar Aug 06 '24 18:08 raynelfss

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core
  • @kevinhartman
  • @mtreinish

qiskit-bot avatar Aug 06 '24 18:08 qiskit-bot

Pull Request Test Coverage Report for Build 10632745058

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 44 of 64 (68.75%) changed or added relevant lines in 2 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 89.179%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/circuit_data.rs 35 55 63.64%
<!-- Total: 44 64
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/two_qubit_decompose.rs 1 90.82%
crates/qasm2/src/lex.rs 4 92.23%
<!-- Total: 5
Totals Coverage Status
Change from base Build 10598013395: -0.02%
Covered Lines: 71660
Relevant Lines: 80355

💛 - Coveralls

coveralls avatar Aug 06 '24 18:08 coveralls