Add rust friendly assign parameters methods
#12913
Conversation
- Accept owned mapping for `from_mapping`.
…arameters_sequence`
|
One or more of the following people are relevant to this code:
|
raynelfss
added
the
Rust
Pull Request Test Coverage Report for Build 10632745058Warning: 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
💛 - Coveralls |
raynelfss
changed the title
assign parameters methods
There was a problem hiding this comment.
Thanks for this Ray. My biggest concern here is in the first review comment - the new interface gives the Python-space forms additional allocations that they didn't need before. I think we ought to either rework the Python forms (which I suspect might be possible) or at least rework the inner interface a little such that those allocations aren't necessary; they logically aren't, since they weren't there before.
| /// Assigns parameters to circuit data based on a mapping of `ParameterUuid` : `Param`. | ||
| /// This mapping assumes that the provided `ParameterUuid` keys are instances | ||
| /// of `ParameterExpression`. |
There was a problem hiding this comment.
It's conventional in Rust documentation to put the assumptions that cause panics in a section called # Panics to really make them stand out - idiomatic Rust is far stricter about errors than Python, where the conventions are more like "just raise an exception". Panics are serious business.
(But also see the other comment - imo it'd be cleaner just to handle the error properly, since it won't cost us anything.)
- Remove initial allocation of `Param` instances within a vec in `assign_parameters_iterable`. - Revert changes in `assign_parameters_mapping`. - Fix error message in `assign_parameter_from_slice`. - Return an error if a `uuid` is not found in `assign_parameters from mapping. Also, return error if a valid uuid is not found to avoid panicking. - Add `clone_ref(py)` method to `Param` to ensure that we use the most efficient cloning methods (either `clone_ref` with gil or copying). - Implement trait `AsRef<Param>` for `Param` to be able to send both owned and non-owned instances to `assign_parameters_inner`.
jakelishman
added
Changelog: None
Summary
Follow up on #12794.
These commits expose some rust-friendly
assign_parametersmethods inCircuitDataallowing us to perform parameter assignments without having call to python directly.Details and comments
The following commits add the methods
assign_paramters_from_sliceandassign_parameters_from_mappingwhich enable the assignment of parameters in rust in a friendly way instead of using hard-Python structures (Py<_>orBound<'_, _>. Here are their use cases:assign_paramters_from_slice: Use whenever a collection ofParamneeds to be assigned. The amount of parameters sent needs to be of the same size as the the number of parameters in theCircuitDatainstance.assign_parameters_from_mapping: Use whenever of mapping betweenParameterUuidinstances from theCircuitDataandParaminstances coming fromGatesorOperations. This can be used for partial assignment, and the number of parameters can be smaller or equal to the amount of parameters in theCircuitDatainstance.Known issues
The mapping ofWill do a mapping betweenParams:Paramsmay not be optimal as we wouldn't know if the types of parameters we're binding are valid,Param::Floatdoes not posses auuidand therefore is not a valid key for the mapping.ParameterUuidandParamto assume that all keys are valid instances with validuuids.The mapping ofNow it accepts any mapping ofassign_parameters_from_mappingdoes not accept instances ofMap<Param, Param>due to param not implementing the hash property in Rust.ParameterUuid:Paramas long as it can be turned into an iterator of type(ParameterUuid, Param).Blockers
get_parameter_by_uuidtoCircuitData#12926Other comments