microsoft / qsharp

Azure Quantum Development Kit, including the Q# programming language, resource estimator, and Quantum Katas
https://microsoft.github.io/qsharp/
MIT License
369 stars 73 forks source link

Fix debugger scope handling during early returns #1528

Closed swernli closed 1 month ago

swernli commented 1 month ago

Changes introduced in #1442 to enable debugger tracking of variable scopes caused a bug if a program used callables with early returns, namely those returns would not pop any added scopes and later execution would read/write the wrong variable states. This fixes the issue by introducing a debug-only return node that triggers a frame leave handling in the environment. This needs to be debug only because it should not be used during partial evaluation.

github-actions[bot] commented 1 month ago

Benchmark for 0afac77

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | Array append evaluation | 344.7±6.37µs | **331.0±10.69µs** | **-3.97%** | | Array literal evaluation | 200.0±4.28µs | **171.4±2.09µs** | **-14.30%** | | Array update evaluation | 434.9±2.47µs | **412.6±1.53µs** | **-5.13%** | | Core + Standard library compilation | 20.5±1.17ms | 20.4±1.30ms | -0.49% | | Deutsch-Jozsa evaluation | **5.1±0.04ms** | 5.2±0.15ms | **+1.96%** | | Large file parity evaluation | 33.9±0.12ms | **33.7±0.54ms** | **-0.59%** | | Large input file compilation | 14.4±0.46ms | **13.7±0.36ms** | **-4.86%** | | Large input file compilation (interpreter) | 51.1±2.08ms | 51.8±1.81ms | +1.37% | | Large nested iteration | 33.8±0.17ms | **33.1±0.24ms** | **-2.07%** | | Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample | 1616.9±159.20µs | 1638.8±211.67µs | +1.35% | | Perform Runtime Capabilities Analysis (RCA) on large file sample | **8.2±0.17ms** | 8.4±0.17ms | **+2.44%** | | Perform Runtime Capabilities Analysis (RCA) on teleport sample | 1495.7±211.15µs | 1487.0±170.75µs | -0.58% | | Perform Runtime Capabilities Analysis (RCA) on the core and std libraries | 29.0±0.28ms | 28.9±0.17ms | -0.34% | | Teleport evaluation | 89.1±5.51µs | 90.1±3.51µs | +1.12% |
swernli commented 1 month ago

Regarding testing, I'm thinking it would make sense to add some targeted unit tests to the debugger tests in this PR and then add execution of all the samples to integration tests. We'll have to decide how to handle the resource estimation samples so the test execution is not too long from trying to simulate a scalable algorithm, but I don't want to delay this PR while we work that out.

github-actions[bot] commented 1 month ago

Benchmark for c4cad6b

Click to view benchmark | Test | Base | PR | % | |------|--------------|------------------|---| | Array append evaluation | 334.6±2.65µs | **326.5±1.73µs** | **-2.42%** | | Array literal evaluation | 186.3±0.78µs | **176.9±1.15µs** | **-5.05%** | | Array update evaluation | 414.1±2.29µs | **408.5±3.75µs** | **-1.35%** | | Core + Standard library compilation | 17.3±0.33ms | **17.0±0.08ms** | **-1.73%** | | Deutsch-Jozsa evaluation | 5.1±0.05ms | **5.0±0.04ms** | **-1.96%** | | Large file parity evaluation | **33.9±0.13ms** | 34.1±0.19ms | **+0.59%** | | Large input file compilation | 12.2±0.28ms | 12.2±0.36ms | 0.00% | | Large input file compilation (interpreter) | 46.7±2.06ms | **43.8±0.82ms** | **-6.21%** | | Large nested iteration | 32.6±0.17ms | 32.5±0.23ms | -0.31% | | Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample | 1567.2±43.55µs | 1550.8±28.09µs | -1.05% | | Perform Runtime Capabilities Analysis (RCA) on large file sample | 7.8±0.06ms | **7.7±0.07ms** | **-1.28%** | | Perform Runtime Capabilities Analysis (RCA) on teleport sample | 1432.9±46.43µs | 1416.5±28.28µs | -1.14% | | Perform Runtime Capabilities Analysis (RCA) on the core and std libraries | 27.4±0.59ms | **27.0±0.15ms** | **-1.46%** | | Teleport evaluation | 87.5±3.76µs | 87.0±3.80µs | -0.57% |