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

[Draft] operator API revision to align with specs #2710

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

Conversation

bettinaheim
Copy link
Collaborator

@bettinaheim bettinaheim commented Mar 7, 2025

I still meant to move the operator files into a separate folder. I'll do that last once I've checked everything else looks good.
I'll also take another look to exhaustively list what really needs to be breaking changes, and what can be further mitigated.

Most of the changes are just renaming + updates to the new API to avoid using what will be deprecated. The meaningful changes are:

  • operators all properly track identities; that means that spin_op::x(10) is no longer considered to be acting on 11 qubits... This is the heaviest breaking change we can't get rid of - it needs to be broken. It is just plain wrong that an operator on a single 2-level degree of freedom does not produce a 2x2 matrix
  • Still to be contemplated: The existing default constructor for spin_op also has the oddity of adding a 1. This too imo is just plain wrong; we see plenty of places where a default was constructed, terms were added, and then the 1 is subtracted again. It also assumed it was acting on degree 0. This default constructor needs to go. The intended constructor for sum_ops is indeed an empty sum. however, because nothing times something is still nothing, that might be surprising and would be breaking existing code pretty heavily. Correspondingly, I made the default protected and added an explicit ::empty (= mathematical 0) and ::identity (= mathematical 1) for construction. Not having a default constructor though would also break code, and we could properly deprecate it...
  • What is done in the simulator files
  • Unification of complex_matrix and matrix_2, now called complex_matrix
  • Align degree vector order with the order that simulators expect
  • Changing the key that the cudaq runtime uses to retrieve expectation values for spin ops
  • two or three minor changes in the signatures of internal functions (spin_op_term vs spin_op, adding missing const)
  • adds proper iterators for product op and sum op, and removes get_terms
  • a bunch of class renaming as requested by the spec

bettinaheim and others added 27 commits March 5, 2025 16:55
…n changes I haven't committed yet

Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
…r cleaner and more predictable

Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Signed-off-by: Bettina Heim <[email protected]>
Early cmake changes necessary (?) for libraries
Copy link

copy-pr-bot bot commented Mar 8, 2025

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.


namespace cudaq {

// FIXME: rename to spin ...
class spin_operator : public operator_handler {
enum class pauli { I, X, Y, Z };
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a different order than the order called out in the comment on line 27. Does that matter? Should the comment or enum be updated?

// I = 0, Z = 1, X = 2, Y = 3

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The order does not matter, and neither ever does the private op code - it is private for a reason. :)
That said, I can align it with this one so that the data format and the internal representation match.

@bettinaheim
Copy link
Collaborator Author

bettinaheim commented Mar 9, 2025

/ok to test

Command Bot: Processing...

@bettinaheim bettinaheim marked this pull request as ready for review March 9, 2025 21:07
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