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

Add for_loop to cudaq::kernel_builder and cudaq.Kernel #19

Merged
merged 5 commits into from
Apr 4, 2023

Conversation

amccaskey
Copy link
Collaborator

@amccaskey amccaskey commented Mar 23, 2023

Enable expressions like this

auto [circuit, inSize] = cudaq::make_kernel<std::size_t>();
auto qubits = circuit.qalloc(inSize);
circuit.h(qubits[0]);
circuit.for_loop(0, inSize - 1, [&](auto &index) {
  circuit.x<cudaq::ctrl>(qubits[index], qubits[index + 1]);
});

and

circuit, inSize = cudaq.make_kernel(int)
qubits = circuit.qalloc(inSize)
circuit.h(qubits[0])
circuit.for_loop(0, inSize-1, lambda index : circuit.cx(qubits[index], qubits[index+1]))
counts = cudaq.sample(circuit, 5)

#53

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.

Nice.

Copy link
Collaborator

@bettinaheim bettinaheim left a comment

Choose a reason for hiding this comment

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

Requesting changes due to missing documentation.

python/runtime/cudaq/builder/py_kernel_builder.cpp Outdated Show resolved Hide resolved
runtime/cudaq/builder/kernel_builder.h Show resolved Hide resolved
@bettinaheim
Copy link
Collaborator

@amccaskey Please create or link the issue with a brief RFC.
A couple of questions off the top of my head:

  • Is there going to be a dedicate loop where we iterate over e.g. an array?
  • What would it look like to iterate over a QuakeValue?
  • Is the iterator always incremented by +1? Why not make it more general, and how would that look like?

@bettinaheim
Copy link
Collaborator

@anthony-santana Please also take a look.

@bettinaheim bettinaheim self-assigned this Apr 3, 2023
@amccaskey amccaskey force-pushed the kernelForLoop branch 2 times, most recently from 798a433 to d7c22e4 Compare April 3, 2023 19:52
@amccaskey
Copy link
Collaborator Author

  • like

The iterator here is just +1 since I'm trying to use existing infrastructure from the bridge. We could of course add the ability to specify the step size, but I think we should file an issue and a separate PR for that.

You can definitely use this to iterate through an existing StdVecType QuakeValue. I have added support and a test for that with the latest commit, please check it out.

I will file the issue for this, which I forgot to do...

@bettinaheim bettinaheim dismissed their stale review April 4, 2023 07:03

The requested changes have been addressed.

@bettinaheim
Copy link
Collaborator

  • like

The iterator here is just +1 since I'm trying to use existing infrastructure from the bridge. We could of course add the ability to specify the step size, but I think we should file an issue and a separate PR for that.

You can definitely use this to iterate through an existing StdVecType QuakeValue. I have added support and a test for that with the latest commit, please check it out.

I will file the issue for this, which I forgot to do...

Thanks. I would suggest to have one issue filed that captures the complete feature (all loop constructs that intend to support in the near future), even if the implementation work is split into several PRs. Let's make sure we don't need to change anything about this builder API to support additional scenarios in the future.

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
@amccaskey amccaskey merged commit 36922cc into NVIDIA:main Apr 4, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2023
@bettinaheim bettinaheim added the release notes Changes need to be captured in the release notes label Apr 11, 2023
@bettinaheim bettinaheim added the enhancement New feature or request label Jun 28, 2023
@amccaskey amccaskey deleted the kernelForLoop branch September 13, 2023 23:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request release notes Changes need to be captured in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants