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.

Sign up for GitHub

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

Jump to bottom

Perf. improvement for CuStateVecCircuitSimulator::observe #1002

Merged
merged 11 commits into from
Mar 21, 2024
Merged

Perf. improvement for CuStateVecCircuitSimulator::observe #1002

merged 11 commits into from
Mar 21, 2024

Conversation

1tnguyen
Copy link
Collaborator

@1tnguyen 1tnguyen commented Dec 5, 2023 •
edited by bettinaheim
Loading

Description

Currently, we don't use the batched Pauli expectation API for multi-term spin_op expectation calculation.

This API is substantially faster than applying change-of-basis gates then observing <ZZZ..ZZ> for each term.

Hence, this PR modifies custatevec backend to use batched Pauli expectation API by default for deterministic cudaq::observe (no shots)

(1) Change CircuitSimulator::observe to return observe_result, which can encapsulate the final expectation value as well as individual terms' expectation values.

(2) Use custatevec batched API and populate the observe_result accordingly.

(3) Add a backward compatibility test for the term-by-term mode.

See also: #1477, #1430

@1tnguyen
Perf. improvement for CuStateVecCircuitSimulator::observe
eebaed2 
This mode is only activated with env. variable
CUDAQ_OBSERVE_FROM_SAMPLING=OFF/0/FALSE.

Currently, we didn't use the batched Pauli APIs and skipped it entirely
for fp-32.

Modify it to use batched Pauli expectation APIs for perf.
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2023

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Dec 5, 2023
@cuda-quantum-bot
Docs preview for PR #1002.
c8ac7da
schweitzpgi
schweitzpgi reviewed Dec 5, 2023
View reviewed changes
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.

runtime/nvqir/custatevec/CuStateVecCircuitSimulator.cu Show resolved Hide resolved
bmhowe23
bmhowe23 reviewed Dec 27, 2023
View reviewed changes
Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

I suspect we need some docs for the CUDAQ_OBSERVE_FROM_SAMPLING environment variable.

@amccaskey
Copy link
Collaborator

@1tnguyen seems like we should also update CircuitSimulator::shouldObserveFromSampling() to return false by default. What do you think? Should this new approach be the default? Is it always faster?

@1tnguyen
Copy link
Collaborator Author

@1tnguyen seems like we should also update CircuitSimulator::shouldObserveFromSampling() to return false by default. What do you think? Should this new approach be the default? Is it always faster?

Yes, it's always faster: a lot faster for large Hamiltonians since the time to run change-of-basis gates and then uncompute them is not insignificant.

There is an issue with making it the default. Currently, we expect that cudaq::observe does return term-by-term results like in this test.
Hence, we need to make an additional change to make CircuitSimulator::observe able to return term-by-term results if it can (like in this case, we perform batch exp-val computation and do have all the individual results). Then, we can make CuStateVecCircuitSimulator::shouldObserveFromSampling return true by default.

What do you think?

@amccaskey
Copy link
Collaborator

@1tnguyen seems like we should also update CircuitSimulator::shouldObserveFromSampling() to return false by default. What do you think? Should this new approach be the default? Is it always faster?

Yes, it's always faster: a lot faster for large Hamiltonians since the time to run change-of-basis gates and then uncompute them is not insignificant.

There is an issue with making it the default. Currently, we expect that cudaq::observe does return term-by-term results like in this test. Hence, we need to make an additional change to make CircuitSimulator::observe able to return term-by-term results if it can (like in this case, we perform batch exp-val computation and do have all the individual results). Then, we can make CuStateVecCircuitSimulator::shouldObserveFromSampling return true by default.

What do you think?

I say go for it - make that change here in this PR.

@github-actions GitHub Actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Jan 30, 2024
@cuda-quantum-bot
Docs preview for PR #1002.
bf60466
@1tnguyen
Merge branch 'main' into tnguyen/cusv-perf
1bf74e4
@1tnguyen
CircuitSimulator::observer to return cudaq::observe_result
debd992 
This allows a fast batched observation while still allowing
per-term data to be progagated up the stack.
@1tnguyen
Remove debug code
5dda027
@1tnguyen
Handle CuStateVecCircuitSimulator::canHandleObserve more generic
f0f7687 
- Use Shots > 0 condition to know if sampling is required.

- Adjust base shouldObserveFromSampling to set default if needed. Plus,
  the environment variable can be used to both turn the feature on or off.
  e.g., for benchmarking purposes.
@github-actions GitHub Actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Mar 18, 2024
@cuda-quantum-bot
Docs preview for PR #1002.
447477b
@github-actions GitHub Actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
@cuda-quantum-bot
Docs preview for PR #1002.
ee1b2f8
@1tnguyen
Copy link
Collaborator Author

This PR is ready for (re-)review. Ping reviewers... :-)

@github-actions GitHub Actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
@cuda-quantum-bot
Docs preview for PR #1002.
098f4c9
bmhowe23
bmhowe23 reviewed Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

This will be a very nice optimization. Are all the interface changes invisible to the end user?

unittests/CMakeLists.txt Show resolved Hide resolved
unittests/backends/qpp_observe/QppObserveTester.cpp Show resolved Hide resolved
bmhowe23
bmhowe23 approved these changes Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

👍

@github-actions GitHub Actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Mar 21, 2024
@cuda-quantum-bot
Docs preview for PR #1002.
2ed2356
@1tnguyen 1tnguyen merged commit c1f0af9 into NVIDIA:main Mar 21, 2024
133 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2024
@bettinaheim bettinaheim added this to the release 0.7.1 milestone Apr 17, 2024
@bettinaheim bettinaheim added the enhancement New feature or request label Apr 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@schweitzpgi schweitzpgi schweitzpgi left review comments

@bmhowe23 bmhowe23 bmhowe23 approved these changes

@amccaskey amccaskey Awaiting requested review from amccaskey

Assignees
No one assigned
Labels
enhancement New feature or request performance
Projects
None yet
Milestone
release 0.7.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@1tnguyen @amccaskey @schweitzpgi @bmhowe23 @bettinaheim