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

Expose hardware prng sequence #515

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

Conversation

erichulburd
Copy link
Contributor

@erichulburd erichulburd commented Nov 25, 2024

This PR supports two new features:

  • A CALL choose_random_real_sub_regions instruction.
  • A RandomizedMeasurement struct / class that will add per-shot randomized measurements to a program, as well as support the client in ascertaining which random unitary was played on each qubit per shot.

The documentation should stand on its own here. One note, I've put all of this functionality into qpu/experimental (both in Rust and Python). It is experimental both in the sense that it is an experimental feature in the QCS SDK that should not be considered stable as well as in the sense that it supports features relevant to QIS experimentalists.

Review Guidance:

  1. Start with crates/lib/src/qpu/experimental/random.rs.
  2. crates/lib/src/qpu/experimental/randomized_measurements.rs measurements builds off the primitives defined in the file above.
  3. Stubs for these features are defined in crates/python/qcs_sdk/qpu/experimental.pyi. Their implementation is defined in crates/python/src/qpu/experimental.pyi.
  4. There are corresponding tests for randomized measurements in crates/lib/src/qpu/experimental/randomized_measurements.rs and crates/python/tests/qpu/test_experimental.py.

@erichulburd erichulburd changed the title Expose hardware prng sequence Draft: Expose hardware prng sequence Nov 25, 2024
@erichulburd erichulburd force-pushed the expose_hardware_prng_sequence branch from 05c6136 to 276ea18 Compare November 25, 2024 16:46
@erichulburd erichulburd changed the title Draft: Expose hardware prng sequence Expose hardware prng sequence Dec 2, 2024
@@ -0,0 +1 @@
crates/lib/src/qpu/experimental @erichulburd
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have push permissions to this repository, which is probably related to the error reported by Github here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Shadow53 did you add Eric since this comment was made? It looks like he should already have admin permission.

crates/lib/src/qpu/experimental/random.rs Show resolved Hide resolved
crates/lib/tests/prng_test_cases.json Show resolved Hide resolved
and copy it to the destination memory region according to the seed value. The seed value is
mutated to the next element in the PRNG sequence.

>>> program = Program()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this doctest as an example because we cannot support a more polished interface (due to the fact that we cannot include quil-rs types in the Python interface). Unfortunately, pytest won't actually run this doctest (it doesn't natively support stub files).

One alternative I considered was implementing some high level functions for using ChooseRandomRealSubRegions natively in Python by importing quil-py types. I stopped short there because there isn't really precedent, however, I think it's probably the best tact.

Copy link
Contributor Author

@erichulburd erichulburd Jan 6, 2025

Choose a reason for hiding this comment

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

I've used pdoc to include test files within the documentation. This has the following advantages:

  1. Example source code is tested.
  2. Examples are only implemented once (rather than repeated b/t docs and tests).
  3. Is compatible with pdoc.
  4. Is compatible with PyO3 modules (I've not been able to get doctests within stubfiles for PyO3 modules to run at all).

Disadvantage:

More testing boilerplate than is ideal.

I've also done this with crates/python/qcs_sdk/qpu/experimental/randomized_measurements.pyi.

crates/python/qcs_sdk/qpu/experimental.pyi Outdated Show resolved Hide resolved
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.

Partial review, will do more later.

crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/externed_call.rs Outdated Show resolved Hide resolved
crates/python/src/qpu/experimental.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/randomized_measurements.rs Outdated Show resolved Hide resolved
crates/python/src/qpu/experimental.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@kalzoo kalzoo left a comment

Choose a reason for hiding this comment

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

Looking good, about ready to approve. I'm as uneasy as before about merging this experimental utility into the SDK itself, but it's the best option we have at the moment because of python binding & distribution. Your take here is neat, compartmentalized, and tested.

Things I'd like to see before merge:

  • A module doc as a general explainer about what's going on here. Doesn't have to specify a higher-level application but should give some context - as it is, I think almost any user might miss the point here.
  • A trustworthy explainer link for LFSR for context
  • A Rust e2e example under examples since it would require live QPU access

Thinking ahead: I'm also wondering if "program builder" or "program operator" is going to be a common pattern going forward and what that might look like. Doesn't block this PR, but I do wonder if a trait might help conceptually group tools like this in the mind of a user and help them understand how they're used:

trait ProgramOperator {
  fn apply(&self, program: Program) -> Result<Program, Self::Error> {
     ...
  }
}

crates/lib/src/qpu/experimental/mod.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/mod.rs Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Show resolved Hide resolved
/// A set of unitaries, each of which may be expressed as a set of Quil
/// instructions.
#[derive(Debug, Clone)]
pub struct UnitarySet(UnitarySetInner);
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought from conversation that your intent was to make this user-definable and not prescriptive. Is that feasible?

Copy link
Contributor Author

@erichulburd erichulburd Jan 2, 2025

Choose a reason for hiding this comment

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

I meant the actual UnitarySet instantiations are up to the user, however, we need to support the impl methods on UnitarySet in order to be able to construct the program properly.

Under this consideration, it's probably best to refactor this as a trait, which can be implemented in Python. The challenge here (and the reason why I originally implemented as an enum) is that the trait really needs quil-rs types in its signature - even from the Python side. I'll give it a go.

Copy link
Contributor Author

@erichulburd erichulburd Jan 6, 2025

Choose a reason for hiding this comment

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

This worked out pretty well (though wasn't trivial). I went ahead and monkey patched a Python ABC class into the Rust module to make the interface clear.

See crates/python/qcs_sdk/_unitary_set.py (and the monkey patching in crates/python/qcs_sdk/__init__.py) and UnitarySetWrapper in crates/python/src/qpu/experimental/randomized_measurements.rs.

crates/python/src/qpu/experimental.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/experimental/random.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/externed_call.rs Outdated Show resolved Hide resolved
crates/lib/src/qpu/externed_call.rs Outdated Show resolved Hide resolved
@erichulburd erichulburd force-pushed the expose_hardware_prng_sequence branch from 67e1b37 to 33af34c Compare January 6, 2025 21:53
@erichulburd
Copy link
Contributor Author

Looking good, about ready to approve. I'm as uneasy as before about merging this experimental utility into the SDK itself, but it's the best option we have at the moment because of python binding & distribution. Your take here is neat, compartmentalized, and tested.

Things I'd like to see before merge:

  • A module doc as a general explainer about what's going on here. Doesn't have to specify a higher-level application but should give some context - as it is, I think almost any user might miss the point here.
  • A trustworthy explainer link for LFSR for context
  • A Rust e2e example under examples since it would require live QPU access

Thinking ahead: I'm also wondering if "program builder" or "program operator" is going to be a common pattern going forward and what that might look like. Doesn't block this PR, but I do wonder if a trait might help conceptually group tools like this in the mind of a user and help them understand how they're used:

trait ProgramOperator {
  fn apply(&self, program: Program) -> Result<Program, Self::Error> {
     ...
  }
}
  • I've added a bit more flesh to the Rust and Python module documentation.
  • The e2e integration test passed for me locally.
  • I think Wikipedia is a good entrypoint for the LFSR algorithm and have linked it.

@erichulburd
Copy link
Contributor Author

I think Wikipedia is a good entrypoint for the LFSR algorithm and have linked it.

It's actually kind of difficult to find a concise and effective summary of LFSR that is not Wikipedia. If we want to avoid linking Wikipedia,: https://pylfsr.github.io/ may be our best best.

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.

4 participants