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

Fix Photonics get_state #2243

Closed
wants to merge 64 commits into from
Closed

Conversation

Omar-ORCA
Copy link
Contributor

Fixes get_state of the photonics simulator

Copy link

copy-pr-bot bot commented Oct 2, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 3, 2024

/ok to test

Command Bot: Processing...

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 4, 2024

/ok to test

Command Bot: Processing...

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 4, 2024

/ok to test

Command Bot: Processing...

3 similar comments
@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 7, 2024

/ok to test

Command Bot: Processing...

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 8, 2024

/ok to test

Command Bot: Processing...

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 8, 2024

/ok to test

Command Bot: Processing...

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 8, 2024

/ok to test

Command Bot: Processing...

github-actions bot pushed a commit that referenced this pull request Oct 8, 2024
Copy link

github-actions bot commented Oct 8, 2024

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

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 9, 2024

/ok to test

Command Bot: Processing...

Copy link

github-actions bot commented Oct 9, 2024

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

github-actions bot pushed a commit that referenced this pull request Oct 9, 2024
@amccaskey
Copy link
Collaborator

One big question I have is what does the PhotonicsCircuitSimulator add beyond what is in CircuitSimulator? I wonder if you could craft a CircuitSimulator subtype that does what you need? I would have thought so if you craft the execution manager to only use the applyCustomUnitary operation.

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 9, 2024

/ok to test

Command Bot: Processing...

github-actions bot pushed a commit that referenced this pull request Oct 9, 2024
Copy link

github-actions bot commented Oct 9, 2024

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

@khalatepradnya
Copy link
Collaborator

khalatepradnya commented Oct 9, 2024

/ok to test

Command Bot: Processing...

Omar-ORCA and others added 20 commits October 11, 2024 09:46
Support for asynchronous sampling for ORCA backend

---------

Co-authored-by: Pradnya Khalate <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
* Start on pure quantum struct usage in kernels

Signed-off-by: Alex McCaskey <[email protected]>

* Update the python bindings with new qstruct restrictions

Signed-off-by: Alex McCaskey <[email protected]>

* Enable default parenthesis constructor

Signed-off-by: Alex McCaskey <[email protected]>

* disallow recursive quantum struct

Signed-off-by: Alex McCaskey <[email protected]>

* Implement error handling for various cases in python

Signed-off-by: Alex McCaskey <[email protected]>

* spell fixes

Signed-off-by: Alex McCaskey <[email protected]>

* forgot to filter out __qpu__ methods on structs, those are allowed

Signed-off-by: Alex McCaskey <[email protected]>

* Add new quantum reference type, !quake.struq, and a couple of new
operations: quake.make_struq and quake.get_member.  These add the
utility of having a product quantum reference type (to logically group
together sets of qubits) but keep the classical and quantum dialects
distinct.

Update the tests, python ast bridge, C++ bridge, add codegen patterns,
etc.

* Whackamole games with the CI.

Add roundtrip test on new type and ops.

Update the python tests. Also change test to eliminate deprecation
warnings.

Add invlid IR checks for new operations.

Add sanity checks. We do not want to allow a quantum struct that holds
anything but non-owning references to qubits or qubit collections.

Remove unused folder pattern.

Workaround for overly assertive compiler warning.

Reenable the hash-and-cache of extract_ref ops in the C++ bridge. This
is a dubious optimization that we may actually want to take out at some
point, but that should be part of a distinct/different PR.

Update test to work around that pytest output can be shuffled.

Add case to python for quake.struq type.  Another python fix.

Add explicit checks to utils.py.

Stab in the dark.

---------

Signed-off-by: Alex McCaskey <[email protected]>
Co-authored-by: Alex McCaskey <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
* Random walk phase estimation example in Python NVIDIA#1579

* Formatting and spelling

* Missed one formatting change

---------

Co-authored-by: Bettina Heim <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
* Fixes a bug with separate compilation.

The bridge wasn't adding all possible `__qpu__` functions to the list, so the
call converter wasn't converting a pure device kernel call on the device side.
This change fixes that bug, updates the call converter to be able to add any
missing declaration(s), and adds a regression test.

Signed-off-by: Eric Schweitz <[email protected]>

* Update the documentation.

Signed-off-by: Eric Schweitz <[email protected]>

---------

Signed-off-by: Eric Schweitz <[email protected]>
Signed-off-by: Omar Bacarreza <[email protected]>
Removed custom operations
Cleaned code
Improve comments and function docs
Signed-off-by: Omar Bacarreza <[email protected]>
@boschmitt
Copy link
Collaborator

boschmitt commented Oct 11, 2024

I cannot make much sense of what is going on in this PR. (It seems that the latest changes broke the diff.) That being said, the title "Fix Photonics get_state" doesn't seems to describe what this is doing: from what I can tell this is adding a whole parallel infrastructure for handling qudit-based computation.

The previous photonics work was supposed to be a stopgap until qudits became better supported in the stack. This PR seems to be working on the opposite direction: instead of improving the current infrastructure, it is creating a parallel one to handle qudits.

The ExecutionManager works on Qudits (QuditInfo). Would it be possible to update the CircuitSimulator to operate on QuditInfo instances? @boschmitt what are your thoughts here? should we keep these things separate?

It seems to me that we don't even need to make CircuitSimulator operate on QuditInfo, we do need to give it the ability to know about qudits, but most of the methods in this new PhotonicCircuitSimulator work on qudit indices, which are exactly the same thing as qubit indices.

The point made about the gates being different is correct, but arguably the simulator should not provide gate methods, e.g. ccx(..) or beam_spliter(...), instead it should have a generic method that takes a matrix and the indices for either qubits or qudits.

Measurements are a bit more annoying but we could make so that CircuitSimulator layer returns a int8_t (or uint8_t), and then we properly cast it to bool when dealing with qubits, e.g., cudaq::mz will have a overload for cudaq::qubit that do the casting before returning to the user.

…users.noreply.github.com>

I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: 7c661ce
I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: d005f25

Signed-off-by: Pradnya Khalate <[email protected]>
….github.com>

I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: cc1acc1

Signed-off-by: Pradnya Khalate <[email protected]>

DCO Remediation Commit for Pradnya Khalate <[email protected]>

I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: 7c661ce
I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: d005f25
I, Pradnya Khalate <[email protected]>, hereby add my Signed-off-by to this commit: 6fc6258

Signed-off-by: Pradnya Khalate <[email protected]>

:code:`plus`
:code:`create`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: Is this a new gate? or an alias for 'plus'? Doesn't match the examples and API spec...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new gate replacing the plus that can be deprecated, the behaviour of the plus gate is not very intuitive (cycling through the number of photons).

plus(q[0]);
create(q[0]);

:code:`annihilate`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

@Omar-ORCA
Copy link
Contributor Author

Superseded by #2283

@Omar-ORCA Omar-ORCA closed this Oct 15, 2024
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.