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

Simulation time bugfix #48

Merged
merged 3 commits into from
Dec 16, 2024
Merged

Simulation time bugfix #48

merged 3 commits into from
Dec 16, 2024

Conversation

mkvanhooft
Copy link
Collaborator

the stack run method did not have a ns.sim_reset() anywhere so multiple calls would not reset simulation time
Generic qdevice was missing S and T gates

@mkvanhooft mkvanhooft requested a review from bvdvecht December 16, 2024 12:31
@mkvanhooft mkvanhooft self-assigned this Dec 16, 2024
@mkvanhooft mkvanhooft requested a review from dieriver December 16, 2024 12:33
@dieriver
Copy link
Collaborator

In general, I don't an issue with this PR.
I checked the NetQASM operation, and it seems they only support S and T gates for NV qprocs. Moreover, in this case, the S and T instructions are transpiled into the equivalent rotations. Assuming that the "Vanilla" flavor of NetQASM maps to the "generic qproc" in squidasm, the vanilla flavor of qprocs do not support S and T gates so far, and it seems there are not NetQASM tests covering those scenarios.
Bottom line: I think this is safe to merge, but I think we should discuss a bit further with Bart once he is back.

@mkvanhooft mkvanhooft removed the request for review from bvdvecht December 16, 2024 14:32
Copy link
Collaborator

@dieriver dieriver left a comment

Choose a reason for hiding this comment

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

Approved

@mkvanhooft mkvanhooft merged commit 7e2b6ba into develop Dec 16, 2024
4 checks passed
@mkvanhooft mkvanhooft deleted the simulation-time-bugfix branch December 16, 2024 14:48
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.

3 participants