QuantumKatas icon indicating copy to clipboard operation
QuantumKatas copied to clipboard

`%kata` magic should use the simulator specified in `@Test` attribute

Open tcNickolas opened this issue 3 years ago • 10 comments

Currently executing a cell with %kata or %check_kata selects the simulator to use in this code - which always uses CounterSimulator. This works for now, since all tests are configured to use either CounterSimulator or QuantumSimulator from which the former is derived, but this will break if we ever define tests that use other simulators.

We should replace this code (and the matching code in %check_kata magic implementation) with logic that would find the @Test attribute, read the simulator configuration from it and use that simulator.

tcNickolas avatar Dec 21 '20 19:12 tcNickolas

Hi @tcNickolas, a couple of questions to understand the scope of this issue :

  • Can custom simulators be used by using @Test attribute? Is it in scope of this PR?
  • For ToffoliSimulator: Currently, it supports StackTraceDisplay functionality. But I am not sure if the tests using ToffoliSimulator would need to display diagnostic information using DumpMachine and DumpRegister callable
  • For ResourcesEstimator and QCTraceSimulator : They currently support StackTraceDisplay functionality. I think it might be fine for this scenario since these simulators provide analysis of the resources needed to run a quantum algorithm. It should make less sense for tests using these simulators to call DumpMachine and DumpRegister callable.

Manvi-Agrawal avatar Apr 02 '21 18:04 Manvi-Agrawal

  • Yes, custom simulators can be used with the @Test attribute; you can see an example of a test using CounterSimulator here, and there is an (unfortunately, internal) example of the @Test attribute used with a simulator that is defined in the same project as the Q# code. I guess you could reproduce this last scenario by copying CounterSimulator source code to the project, renaming it and using that in tests.
  • For non-full-state simulators, their tests will only use the functionality they provide, so if a simulator doesn't provide some functionality, the tests won't use them. If they support StackTraceDisplay, there probably will be a scenario in which either the tests will try to be helpful and use DumpMachine, or the learner themselves will try to use it in their code to debug the solution.

P.S. Apologies for not reviewing your other PR yet! I'm heads down on prep for https://www.qchack.io/ starting next week :-(

tcNickolas avatar Apr 02 '21 20:04 tcNickolas

Hi @tcNickolas, no issues for delayed review. I understand that it requires a lot of efforts goes into planning what tutorials and challenges would go into QCHack event. Looking forward to the event. Meanwhile, I think I would need to revisit a few things in the code to do this task after this clarification.

Manvi-Agrawal avatar Apr 03 '21 04:04 Manvi-Agrawal

Hi @tcNickolas , kindly find the design document for this issue.

This might be a bit lengthy, but last 3 pages aren't that important, so you might want to skip them :-) They contain some nitty-gritty details about the code and some other things.

Manvi-Agrawal avatar Apr 26 '21 06:04 Manvi-Agrawal

Hi @Manvi-Agrawal! Your design document looks good to me. For 3a, determining the simulators specified, this iqsharp code should be useful reference. In that line we're retrieving details about documentation, but it should be similar logic to retrieve the specified simulator so that it can be instantiated through reflection.

anjbur avatar May 05 '21 01:05 anjbur

Thank you @anjbur for providing the reference to the iqsharp code.

Manvi-Agrawal avatar May 05 '21 04:05 Manvi-Agrawal

Hi @anjbur @tcNickolas are we thinking to update the CI to use the latest version of the katas package and then merge this? CI updation would require some discussion so thinking to do in a separate PR.

Or do we proceed by ensuring things work locally

Manvi-Agrawal avatar May 10 '21 07:05 Manvi-Agrawal

Validating changes to the magic packages is always a challenge... I aim to always keep the Katas on a publicly available QDK version, so that anybody installing them doesn't get exciting errors stemming from the use of packages that were not released yet.

I would be very reluctant to continue digging in the direction of #621, since any changes to Dockerfile and scripts used in build have the potential to break the pipelines more than to validate the new package behavior. For example, the things we do for NuGet.config with the <clear/> element ensure that the builds don't try to re-download NuGet packages beyond the first time they do that to cut down the build time - it's possible that Binder timeouts you see are caused by Binder trying to re-download them, or it could be a different issue.

I would recommend to do a lot of careful manual validation for #620, then merge it with the understanding that the effects will not be observable until the next QDK release in the end of month, and then (after more manual validation that is part of the release process) switch to this change with the next release.

tcNickolas avatar May 10 '21 17:05 tcNickolas

Hi @tcNickolas @anjbur , makes sense to not change Dockerfile and the validate-notebooks.ps1 script since they might cause the build failures. I would like to know if we want to retain the Prototype notebook or not as mentioned in https://github.com/microsoft/QuantumKatas/pull/621

Manvi-Agrawal avatar May 10 '21 18:05 Manvi-Agrawal

I would recommend to do a lot of careful manual validation for #620, then merge it with the understanding that the effects will not be observable until the next QDK release in the end of month, and then (after more manual validation that is part of the release process) switch to this change with the next release.

Hi @tcNickolas. How much manual testing are you expecting for #620? I have tested the effects of changes of CheckKataMagic for all notebooks using %check_kata locally. So, do you want exhaustive testing for KataMagic too? I think that might not be very necessary since KataMagic uses logic almost similar to the CheckKataMagic. The only difference was the rich jupyter state display for the QuantumSimulator and CounterSimulator plus the logging messges for all simulators. For that difference, Prototype kata serves as a good aid. Apart from that I ran these tasks locally

  • StateFlip of BasicGates kata : uses DumpMachine and is simulated on QuantumSimulator
  • MainDiagonal of UnitaryPatterns kata : simulated on CounterSimulator

Manvi-Agrawal avatar May 11 '21 08:05 Manvi-Agrawal

We will be migrating the Katas to the new QDK that will use a different infrastructure (see https://devblogs.microsoft.com/qsharp/introducing-the-azure-quantum-development-kit-preview/ for the announcement), including a much faster simulator, so we won't continue to improve the existing infrastructure of Q# notebooks magics. Closing this issue as "won't fix".

tcNickolas avatar Sep 28 '23 23:09 tcNickolas

Thanks @tcNickolas and @anjbur for your valuable guidance.

Looking forward to the new kata experience. Just had a look at https://quantum.microsoft.com/en-us/experience/quantum-katas, and the experience looks way more polished :-)

Manvi-Agrawal avatar Oct 12 '23 01:10 Manvi-Agrawal