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

[StateHandling] State API for remote platforms #1653

Conversation

1tnguyen
Copy link
Collaborator

@1tnguyen 1tnguyen commented May 10, 2024

Description

State handling for remote platforms:

  • Introduce SimulationState subclass for remote platform (RemoteSimulationState and PyRemoteSimulationState) which is essentially a holder of the kernel IR.

  • Perform a lazy evaluation to resolve the state:

    (1) For state accessor, e.g., amplitudes: adding a configurable qubit number threshold. If the number of qubits is less than the threshold, resolve the full state vector to the client. Otherwise, send on amplitude access request. Adding an API to cudaq::state for batched amplitude access.

    (2) For overlap, sending job requests with both kernel IRs.

Note: the remote state is implemented for MLIR kernels only (C++ MLIR mode, kernel builder and Python).
Library mode C++ kernel is evaluated as-is (v0.7 mode): state is resolved to a vector by the server for each get_state call.

@1tnguyen 1tnguyen changed the title State API for remote platforms [StateHandling] State API for remote platforms May 10, 2024
- Add multiple amplitude accessor

- Remote implementation of multiple amplitude access.

- Code tidy up
github-actions bot pushed a commit that referenced this pull request May 23, 2024
@1tnguyen 1tnguyen marked this pull request as ready for review May 23, 2024 06:38
github-actions bot pushed a commit that referenced this pull request May 23, 2024
Copy link
Collaborator

@schweitzpgi schweitzpgi left a comment

Choose a reason for hiding this comment

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

LGTM. You'll need additional approval though.

python/runtime/cudaq/algorithms/py_state.cpp Outdated Show resolved Hide resolved
namespace cudaq {

void pyAltLaunchKernel(const std::string &, MlirModule, OpaqueArguments &,
const std::vector<std::string> &);

std::tuple<cudaq::ArgWrapper, std::size_t, std::int32_t>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this tuple have a name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good idea. Added a struct in c497846 to name this tuple.

runtime/common/SimulationState.h Show resolved Hide resolved
@@ -27,6 +27,7 @@ int main() {
assert(std::abs(state[2].real()) < 1e-3);
assert(std::abs(M_SQRT1_2 - state[3].real()) < 1e-3);
}

// Skipped test due to a stability issue. See:
// https://github.com/NVIDIA/cuda-quantum/issues/1087
#if 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: delete this block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll keep this block since I haven't had a chance to retest it according to the ticket linked. We probably want to re-enable the test if it's no longer unstable.

runtime/cudaq/algorithms/get_state.h Show resolved Hide resolved
runtime/cudaq/algorithms/get_state.h Show resolved Hide resolved
runtime/cudaq/algorithms/get_state.h Outdated Show resolved Hide resolved
github-actions bot pushed a commit that referenced this pull request Jun 3, 2024
@1tnguyen
Copy link
Collaborator Author

1tnguyen commented Jun 3, 2024

Since this would introduce a breaking change to NVQC, it would be rebased to target main branch after the experimental branch is merged.

@1tnguyen 1tnguyen added the breaking change Change breaks backwards compatibility label Jun 3, 2024
@@ -184,7 +184,7 @@ jobs:
create_function_result=$(ngc-cli/ngc cloud-function function create \
--container-image nvcr.io/${{ env.NGC_QUANTUM_ORG }}/${{ env.NGC_QUANTUM_TEAM }}/cuda-quantum:nightly \
--container-environment-variable NUM_GPUS:1 \
--container-environment-variable NVQC_REST_PAYLOAD_VERSION:1 \
--container-environment-variable NVQC_REST_PAYLOAD_VERSION:2 \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I forget - was this change going to get pulled into a separate PR? Or was it just that we were going to hold off on this full PR until closer to the release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We will close this PR and migrate the changes to a new branch based on main with deployment to test the integration. The changes will be merged closer to release.

@1tnguyen
Copy link
Collaborator Author

Superseded by #1803

@1tnguyen 1tnguyen closed this Jun 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change Change breaks backwards compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants