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
Amplitude state array #45
Conversation
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.
Looks great! 🚀
Only one thing: I think it might be good to add a test for the "dimension" option as well, using a cutoff value larger than 2.
@@ -118,7 +118,8 @@ def run_xir_program(program: XIRProgram) -> List[Union[np.number, np.ndarray]]: | |||
|
|||
num_wires = len(program.wires) | |||
# TODO: Extract the Fock cutoff dimension from the XIR script. |
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 TODO can now be removed, right? Or, perhaps even better, exchanged with a comment saying that a qubit circuit is assumed if no "dimension" is set.
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.
Not quite. I experimented with using options
in this PR but found that the implementation was not easy to get right. Specifically, we'll need to validate that each gate instance is compatible with the dimension of the circuit (which may involve a change to each Gate
subclass). It would also be nice to allow implicit cutoff parameters too; however, this is also non-trivial without some kind of reflection hack.
That being said, I am currently working on a separate PR to address these points and remove the TODO 🙂.
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.
Update: The PR is #47.
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.
Looks great! 🚀
Thanks @thisac!
Only one thing: I think it might be good to add a test for the "dimension" option as well, using a cutoff value larger than 2.
As I mentioned in the review, I would prefer to do this in a future PR since it is a little tricky to implement correctly.
@@ -118,7 +118,8 @@ def run_xir_program(program: XIRProgram) -> List[Union[np.number, np.ndarray]]: | |||
|
|||
num_wires = len(program.wires) | |||
# TODO: Extract the Fock cutoff dimension from the XIR script. |
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.
Not quite. I experimented with using options
in this PR but found that the implementation was not easy to get right. Specifically, we'll need to validate that each gate instance is compatible with the dimension of the circuit (which may involve a change to each Gate
subclass). It would also be nice to allow implicit cutoff parameters too; however, this is also non-trivial without some kind of reflection hack.
That being said, I am currently working on a separate PR to address these points and remove the TODO 🙂.
Context:
To support the remote execution of XIR programs using Jet, the interpreter used an integral representation of an amplitude state; however, since #28, this is no longer necessary and amplitude states should instead be specified as arrays.
Description of the Change:
state
parameter ofamplitude
statements to be an array.Benefits:
Possible Drawbacks:
None.
Related GitHub Issues:
None.