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 global phase for PauliI rotation and DumpRegister #1461

Merged
merged 4 commits into from
May 6, 2024

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented May 1, 2024

This change introduces a new, stdlib-only intrinsic for applying global phase during simulation to make phase behavior more consisten and fixes #1450. This intrinsic only applies the global phase during simulation; the new GlobalPhase intrinsic is a no-op for QIR generation, circuit generation, and resource estimation. The change also includes updates to the stdlib and associated tests to account for the corrected global phase behavior.

This also fixes a bug in DumpRegister related to iteration through the state vector that ensures the "base" amplitude used for separating the state is consistently chosen, which avoids corner cases that could introduce a phase in the displayed output that wasn't present in the underlying state.

This change introduces a new, stdlib-only intrinsic for applying global phase during simulation to make phase behavior more consisten and fixes #1450. This intrinsic only applies the global phase during simulation; the new `GlobalPhase` intrinsic is a no-op for QIR generation, circuit generation, and resource estimation. The change also includes updates to the stdlib and associated tests to account for the corrected global phase behavior.

This also fixes a bug in `DumpRegister` related to iteration through the state vector that ensures the "base" amplitude used for separating the state is consistently chosen, which avoids corner cases that could introduce a phase in the displayed output that wasn't present in the underlying state.
@swernli swernli mentioned this pull request May 1, 2024
11 tasks
Copy link

github-actions bot commented May 1, 2024

Change in memory usage detected by benchmark.

Memory Report for 05e923e

Test This Branch On Main Difference
compile core + standard lib 15892349 bytes 15969785 bytes -77436 bytes

@swernli swernli requested a review from tcNickolas May 1, 2024 22:30
Copy link

github-actions bot commented May 1, 2024

Benchmark for 05e923e

Click to view benchmark
Test Base PR %
Array append evaluation 341.3±2.21µs 363.4±3.40µs +6.48%
Array literal evaluation 192.3±0.51µs 195.3±1.95µs +1.56%
Array update evaluation 424.4±2.74µs 429.1±1.91µs +1.11%
Core + Standard library compilation 16.6±0.20ms 17.7±0.72ms +6.63%
Deutsch-Jozsa evaluation 5.1±0.05ms 5.0±0.04ms -1.96%
Large file parity evaluation 33.7±0.07ms 33.5±0.25ms -0.59%
Large input file compilation 11.2±0.30ms 12.0±0.15ms +7.14%
Large input file compilation (interpreter) 43.2±1.13ms 47.7±1.37ms +10.42%
Large nested iteration 33.4±0.09ms 33.5±0.13ms +0.30%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1490.0±26.95µs 1480.4±29.15µs -0.64%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.14ms 7.6±0.09ms -2.56%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1401.4±29.61µs 1403.1±43.75µs +0.12%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 21.0±0.28ms 21.3±0.25ms +1.43%
Teleport evaluation 89.1±3.52µs 87.4±3.71µs -1.91%

@swernli
Copy link
Collaborator Author

swernli commented May 1, 2024

@tcNickolas Ok, with the combination of these changes and the teleport sample update from #1456, I've confirmed a few things of note:

  • The teleportation sample now always returns consistent before and after dumps with the expected phases.
  • R(PauliI) behaves as an application of global phase consistent with how it is documented.
  • R1 no longer introduces an extra global phase and is consistent with the other rotations like Z and S rather than being off by a phase (and introduced unit tests to make sure it stays that way).
  • Application of controlled global phase behave as expected in simulation and introduce calls to Rz where appropriate so that execution on physical systems is consistent.
  • The updated state preparation tests confirm that calls PreparePureStateD now results in states without complex phase that match the inputs provided.
  • The fixes to R1 exposed a bug in the ApplyQFT test cases that, when corrected, confirms the Fourier transform is being performed as expected.

There is still one other issue remaining from #1317 that will require changes to the internals of the simulator, which will be in another PR (hopefully out soon).

Copy link
Member

@minestarks minestarks left a comment

Choose a reason for hiding this comment

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

The actual global phase fix is beyond my knowledge, but the evaluator/backend bits LGTM

Copy link

github-actions bot commented May 2, 2024

Change in memory usage detected by benchmark.

Memory Report for 329443a

Test This Branch On Main Difference
compile core + standard lib 15892349 bytes 15969785 bytes -77436 bytes

Copy link

github-actions bot commented May 2, 2024

Benchmark for 329443a

Click to view benchmark
Test Base PR %
Array append evaluation 340.4±4.39µs 332.0±15.24µs -2.47%
Array literal evaluation 173.9±3.30µs 177.6±6.84µs +2.13%
Array update evaluation 422.8±19.40µs 411.3±8.78µs -2.72%
Core + Standard library compilation 17.9±0.74ms 19.0±0.83ms +6.15%
Deutsch-Jozsa evaluation 5.1±0.06ms 4.9±0.12ms -3.92%
Large file parity evaluation 33.8±0.23ms 33.1±0.75ms -2.07%
Large input file compilation 12.0±0.48ms 12.2±0.51ms +1.67%
Large input file compilation (interpreter) 47.4±1.75ms 48.7±1.39ms +2.74%
Large nested iteration 33.2±0.46ms 32.6±0.73ms -1.81%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1493.8±65.37µs 1494.4±57.27µs +0.04%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.18ms 7.5±0.24ms -3.85%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1407.1±65.00µs 1389.3±96.96µs -1.27%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 21.3±0.49ms 22.1±0.60ms +3.76%
Teleport evaluation 88.5±3.87µs 87.6±5.08µs -1.02%

Copy link

github-actions bot commented May 6, 2024

Change in memory usage detected by benchmark.

Memory Report for ea0a767

Test This Branch On Main Difference
compile core + standard lib 15889189 bytes 15966617 bytes -77428 bytes

Copy link

github-actions bot commented May 6, 2024

Benchmark for ea0a767

Click to view benchmark
Test Base PR %
Array append evaluation 350.8±7.96µs 341.1±2.67µs -2.77%
Array literal evaluation 189.4±0.73µs 179.7±3.37µs -5.12%
Array update evaluation 424.3±3.64µs 421.5±1.63µs -0.66%
Core + Standard library compilation 20.0±0.67ms 20.1±0.65ms +0.50%
Deutsch-Jozsa evaluation 5.2±0.06ms 5.1±0.07ms -1.92%
Large file parity evaluation 33.6±0.08ms 33.8±0.24ms +0.60%
Large input file compilation 13.1±0.39ms 12.8±0.33ms -2.29%
Large input file compilation (interpreter) 50.4±2.35ms 48.8±0.96ms -3.17%
Large nested iteration 33.6±0.13ms 33.4±0.39ms -0.60%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1526.9±85.55µs 1519.7±108.51µs -0.47%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.1±0.10ms 8.0±0.08ms -1.23%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1442.6±101.34µs 1442.8±134.36µs +0.01%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 21.5±0.22ms 21.1±0.20ms -1.86%
Teleport evaluation 90.6±5.60µs 88.3±3.63µs -2.54%

@swernli swernli enabled auto-merge May 6, 2024 20:08
@swernli swernli added this pull request to the merge queue May 6, 2024
Merged via the queue into main with commit fc4ebf4 May 6, 2024
17 checks passed
@swernli swernli deleted the swernli/r-paulii-phase branch May 6, 2024 21:00
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.

Use global phase intrinsic for R with PauliI to make application of phase more consistent
6 participants