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

Fixes Magic Getting Erased by Format in Notebooks #1310

Merged
merged 3 commits into from
Mar 26, 2024

Conversation

ScottCarda-MS
Copy link
Contributor

@ScottCarda-MS ScottCarda-MS commented Mar 25, 2024

This was caused by two different bugs that are addressed in this PR:

  • When the input Q# uses \r\n line endings, the raw token lexer would only consider the \n line ending, resulting in comment tokens with trailing \r characters in them.
  • In the notebooks, when there were newlines prior to the magic command, the offsets were not mapped correctly.

In order to preserve the magic command in the notebooks from getting removed as leading whitespace in a file (a formatter rule), the magic command is replaced by a comment instead of whitespace.

Fixes #1308

fixed bug in notebooks
Copy link

Benchmark for c80daab

Click to view benchmark
Test Base PR %
Array append evaluation 359.1±1.91µs 329.5±4.09µs -8.24%
Array literal evaluation 196.2±5.57µs 193.0±2.83µs -1.63%
Array update evaluation 440.3±1.37µs 413.5±2.81µs -6.09%
Deutsch-Jozsa evaluation 5.3±0.07ms 5.2±0.14ms -1.89%
Large file parity evaluation 33.7±0.10ms 33.6±0.06ms -0.30%
Large input file 34.2±0.70ms 31.6±1.14ms -7.60%
Large nested iteration 35.5±0.96ms 32.3±0.24ms -9.01%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1613.1±169.02µs 1588.5±136.25µs -1.53%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.15ms 7.8±0.07ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1510.9±141.75µs 1512.9±73.93µs +0.13%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 26.6±0.30ms 25.8±0.16ms -3.01%
Standard library 18.3±0.59ms 16.8±0.27ms -8.20%
Teleport evaluation 82.3±4.05µs 80.9±5.23µs -1.70%

Copy link

Benchmark for bef7150

Click to view benchmark
Test Base PR %
Array append evaluation 330.4±10.30µs 343.6±2.25µs +4.00%
Array literal evaluation 195.9±0.92µs 173.5±4.57µs -11.43%
Array update evaluation 413.9±7.08µs 416.9±2.53µs +0.72%
Deutsch-Jozsa evaluation 5.0±0.04ms 5.1±0.16ms +2.00%
Large file parity evaluation 33.8±0.14ms 33.7±0.10ms -0.30%
Large input file 29.4±1.55ms 28.6±0.88ms -2.72%
Large nested iteration 32.3±0.62ms 32.7±0.25ms +1.24%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1613.7±150.20µs 1566.4±57.39µs -2.93%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.14ms 7.8±0.07ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1536.9±63.40µs 1492.9±83.60µs -2.86%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 21.6±0.29ms 21.3±0.14ms -1.39%
Standard library 16.7±0.25ms 16.7±0.37ms 0.00%
Teleport evaluation 81.9±4.00µs 81.1±3.99µs -0.98%

@ScottCarda-MS ScottCarda-MS marked this pull request as ready for review March 26, 2024 17:44
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

Maybe a dumb question, but looking at the test, does this end up replacing %%qsharp with //qsharp in notebooks?

@ScottCarda-MS
Copy link
Contributor Author

Maybe a dumb question, but looking at the test, does this end up replacing %%qsharp with //qsharp in notebooks?

No, the text found in the notebook is only changed by the edits the formatter makes. Since there is nothing wrong format-wise with //qsharp, no format edits are issues to change that text, and the original %%qsharp found in the notebook goes unchanged.

Copy link

Benchmark for 12728e9

Click to view benchmark
Test Base PR %
Array append evaluation 427.4±4.64µs 342.4±1.57µs -19.89%
Array literal evaluation 193.5±3.53µs 174.6±5.71µs -9.77%
Array update evaluation 523.3±2.73µs 427.8±2.72µs -18.25%
Deutsch-Jozsa evaluation 5.4±0.05ms 5.1±0.05ms -5.56%
Large file parity evaluation 33.9±0.12ms 33.5±0.12ms -1.18%
Large input file 29.2±0.75ms 30.3±1.26ms +3.77%
Large nested iteration 43.6±0.32ms 33.6±0.14ms -22.94%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1604.9±82.26µs 1642.2±133.45µs +2.32%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.15ms 7.7±0.07ms -1.28%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1526.8±60.64µs 1522.1±74.87µs -0.31%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 21.4±0.19ms 21.6±0.99ms +0.93%
Standard library 16.5±0.24ms 16.9±0.88ms +2.42%
Teleport evaluation 88.4±4.84µs 83.9±6.17µs -5.09%

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.

Formatting Q# cell erases %%qsharp magic
3 participants