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

feat!: Shot count is now represented as an unsigned 32-bit, up from 16-bit, allowing for higher shot counts #509

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

MarquessV
Copy link
Contributor

closes #332

Copy link
Contributor

@Shadow53 Shadow53 left a comment

Choose a reason for hiding this comment

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

Looks like a few u16s were missed, at least according to linting results.

@@ -108,7 +108,7 @@ impl PyExecutable {
quil: String,
registers: Vec<String>,
parameters: Vec<PyParameter>,
#[pyo3(from_py_with = "crate::from_py::optional_non_zero_u16")] shots: Option<NonZeroU16>,
#[pyo3(from_py_with = "crate::from_py::optional_non_zero_u16")] shots: Option<NonZeroU32>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Linting failure on crate::from_py::optional_non_zero_u16. Should probably be this?

Suggested change
#[pyo3(from_py_with = "crate::from_py::optional_non_zero_u16")] shots: Option<NonZeroU32>,
#[pyo3(from_py_with = "crate::from_py::optional_non_zero_u32")] shots: Option<NonZeroU32>,

@@ -123,7 +123,7 @@ impl PyMultishotRequest {

#[setter]
pub fn set_trials(&mut self, trials: u16) -> PyResult<()> {
// `NonZeroU16` doesn't implement `PyClass`, so `pyo3` doesn't allow it to be used
// `NonZeroU32` doesn't implement `PyClass`, so `pyo3` doesn't allow it to be used
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like u16 is being used in this area (fn trials and fn set_trials) when it should be u32

(Commenting here because GH does not let me comment on non-diff lines)

Copy link
Contributor

Choose a reason for hiding this comment

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

This also happens below in functions with the same names.

Copy link

github-actions bot commented Oct 23, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://rigetti.github.io/qcs-sdk-rust/pr-preview/pr-509/
on branch qcs-sdk-python-docs at 2024-10-24 17:55 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NonZeroU16 is unecessarily restrictive for QVM trials
2 participants