Implement a builder-like constructor for Target#14308
Conversation
|
One or more of the following people are relevant to this code:
|
jakelishman
left a comment
There was a problem hiding this comment.
Rust doesn't have keyword arguments, so a new method that's 8 Option<T>s long perhaps isn't the cleanest user interface. Perhaps a builder-like interface would be cleaner?
let mut target = Target::new()
.with_description("target")
.with_num_qubits(4)
.with_dt(10e-9);or such?
Pull Request Test Coverage Report for Build 21005445683Details
💛 - Coveralls |
6613a3f to
7a9d177
Compare
Target::new() to the rest of the cratesTarget
|
Out of curiosity, would it be easier to just have the constructor setter methods for let mut target = Target::new().try_with_num_qubits(5)?.try_with_qubit_properties(vec![])?;
target.dt = Some(10e-9);
target.min_length = 2; |
| /// let mut target = Target::new().with_qubit_properties(vec![]); | ||
| /// assert_eq!(target.num_qubits, Some(0)); | ||
| /// ``` | ||
| pub fn with_qubit_properties(self, qubit_properties: Vec<PyObject>) -> Self { |
There was a problem hiding this comment.
Is this actually a thing we should have in the rust builder api? It doesn't seem like it's something we'll ever define in Rust while it's a PyObject.
There was a problem hiding this comment.
Not until we have qubit properties in Rust. We may want to have this ready as a placeholder though.
There was a problem hiding this comment.
Do we have an issue tracking the move of qubit properties to rust?
| /// | ||
| /// Args: | ||
| /// operation (str): The operation name to get qargs for | ||
| /// Returns: |
There was a problem hiding this comment.
Is this correct? In sphinx-napoleon/google style which we use for qiskit it should be Returns:. See:https://www.sphinx-doc.org/en/master/usage/extensions/napoleon.html
There was a problem hiding this comment.
Will change it back to Returns:
|
Given the timeframe and that this is a good to have internal usability thing I deferred this 2.2 to minimize the risk to 2.1. |
|
Basically the same reason to defer to 2.4 given that we're 2 days off RC1:
|
da9a36d to
0765275
Compare
Add: Builder-like construction methods. - Add detailed docstrings with warnings about where not to use them. Add: Use builder-like construction in `py_new` Fix: Separate result-handlling and panic versions of `num_qubit` and `qubit_properties` initializers. Test: Add rust unit tests. FIx: Address review comments Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Chore: Use `Returns:` instead of `Returns` in docstrings Fix: Adapt to c changes
Add: Builder-like construction methods. - Add detailed docstrings with warnings about where not to use them. Add: Use builder-like construction in `py_new` Fix: Separate result-handlling and panic versions of `num_qubit` and `qubit_properties` initializers. Test: Add rust unit tests. FIx: Address review comments Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Chore: Use `Returns:` instead of `Returns` in docstrings Fix: Adapt to c changes
- Add detailed docstrings with warnings about where not to use them.
…`qubit_properties` initializers.
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
The following commit removed `with_num_qubits()`, `with_qubit_properties()`. in favor of the fallible methods. Methods have been updated to use these additions.
cac6199 to
98f6e21
Compare
Summary
In an oversight during #14228, we forgot to expose the
Target::new()method to the rest of the crate. The following commits expose a Rust nativeTarget::new()as well as some other methods which enable us to construct theTargetusing a builder-like architecture.Details
The following commits add a rust native constructor for the
Targetthat uses a builder-like architecture to add all of the specified attributes in a more dynamic manner.The methods include the following:
new- Rust native constructor, must be called before using any builder methods. Equivalent toDefault::default().with_description- To create a target with a specific description.with_num_qubits- (Fallible) Assigns an explicit number of qubits to theTarget, may fail if the specifiedqubit_propertieslist length differs from the value of this number.with_dt- Assigns thedtvalue.with_granularity- Assigns a granularity value in units ofdt.with_min_length- Assigns a minimal pulse length in units ofdt.with_pulse_alignment- Assigns a gate instruction starting time.with_acquire_alignment- Assigns a measure instruction starting time.with_qubit_properties- (Fallible) Specifies the properties of each of the backend's qubits, if associated. May fail if the specifiedqubit_propertieslist length differs from the value ofnum_qubits.with_concurrent_measurements- Specifies which qubits need to be measured together.