-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Tensor from classical action #1514
Tensor from classical action #1514
Conversation
@mpharrigan ptal! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sweeeeeeet
A couple of nits.
Do you want to change some of the existing bloqs to use this method? Maybe And
. This can be in a follow-up. I'm also curious in your exhaustive testing notebook. Maybe you can contribute that to dev_tools
in a follow up
for it when the bloq has a known classical action. | ||
|
||
Examples: | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you check how this shows up in the docs? does it look ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this didn't work actually, so I removed the Examples:
. works now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for checking! (I suspected the "Examples" heading wouldn't work)
n_qubits_left = sum(left_qubit_counts) | ||
n_qubits_right = sum(reg.total_bits() for reg in bloq.signature.rights()) | ||
|
||
matrix = np.zeros((2,) * (n_qubits_right + n_qubits_left)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider adding a check with a helpful error message if n_qubits_right + n_qubits_left is an unreasonably large number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raises a value error when total > 40
try: | ||
matrix = _bloq_to_dense_via_classical_action(bloq) | ||
except ValueError as e: | ||
raise ValueError(f"cannot compute tensor: {bloq} is not classical") from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm ValueError could potentially come from another source, no? Maybe include the original str(e)
message in this error message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the improvements!
my_tensors_from_classical_action
: a drop-in replacement forbloq.my_tensors
that authors of classical bloqs can call, to avoid manually defining the explicit tensor.