microsoft / qsharp-runtime

Runtime components for Q#
https://docs.microsoft.com/quantum
MIT License
285 stars 93 forks source link

Runtime fails without error when running operation without body #616

Open tcNickolas opened 3 years ago

tcNickolas commented 3 years ago

Describe the bug

If Q# code uses an operation that is defined as body intrinsic without an actual body implemented by the simulator, running this code will fail, but it will produce no useful error message.

To Reproduce

I used a custom simulator similar to CounterSimulator, but with body for one of the operations not defined. I used it to run a unit test defined using @Test attribute, and used the operation with missing body in it.

Expected behavior

An error message telling me that operation X is not implemented.

Actual behavior

The test fails without any error message.

System information

tcNickolas commented 3 years ago

This behavior persists in QDK version 0.17.2105143879.

bettinaheim commented 3 years ago

@tcNickolas Could you share what exactly the output is that you are getting, and what exactly your setup is? Intrinsic operations will be translated to abstract classes, so for the C# build to succeed, something will have to provide some implementation. Based on the description, I am not sure what that is in your case.

swernli commented 3 years ago

If I try this locally I do get an error when using dotnet run:

> dotnet run
Unhandled exception: System.MemberAccessException: Cannot create an instance of Program.F because it is an abstract class.
   at System.Reflection.RuntimeConstructorInfo.CheckCanCreateInstance(Type declaringType, Boolean isVarArg)  
   at System.Reflection.RuntimeConstructorInfo.ThrowNoInvokeException()
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)

Could this exception be getting swallowed by the test infrastructure somehow?

tcNickolas commented 3 years ago

I have a test defined using @Test attribute which I run in the Visual Studio ("dotnet test" via command line doesn't have error message either).

For the simplest repro of my scenario, you can use the Measurements kata and replace the simulator for T105_ZeroZeroOrOneOne with QuantumSimulator; the test will fail without an error message.

swernli commented 3 years ago

Ah, I see the issue. The unit test infrastructure indeed swallows the error. This is because it is an unhandled exception that is hit at runtime, which is expected, and outputs to stderr and not stdout. If you take a look at the way a Q# tests is generated into C# you'll see where exceptions get ignored:

            [Xunit.Fact()]
            [Xunit.Trait("Target", "QuantumSimulator")]
            [Xunit.Trait("Name", "T105_ZeroZeroOrOneOne")]
            public void T105_ZeroZeroOrOneOne()
#line 224 "C:\\Users\\swern\\Programming\\QuantumKatas\\Measurements\\Tests.qs"
            {
                var sim = new Microsoft.Quantum.Simulation.Simulators.QuantumSimulator();
                if (sim is Microsoft.Quantum.Simulation.Common.SimulatorBase baseSim && this.Output != null)
                {
                    baseSim.OnLog += this.Output.WriteLine;
                }

                try
                {
                    sim.Execute<T105_ZeroZeroOrOneOne, QVoid, QVoid>(QVoid.Instance);
                }
                catch
                {
#line 224 "C:\\Users\\swern\\Programming\\QuantumKatas\\Measurements\\Tests.qs"
                    Xunit.Assert.True(false, "Q# Test failed. For details see the Standard output below.");
                }
                finally
                {
                    if (sim is IDisposable disposeSim)
                    {
                        disposeSim.Dispose();
                    }
                }
            }
        }

That try-catch does not output any exception information, so any runtime exception in a test would get suppressed this way, not just those related to missing intrinsics.

ScottCarda-MS commented 3 years ago

I've tried out @swernli's suggestion, and I don't think that gets us the information that is asked for here. The exception, if caught, is specific to the generated C#, not the Q#, and displaying information from that exception will only add to the confusion. Having something like System.MemberAccessException : Cannot create an instance of Input.No because it is an abstract class. would be especially confusing to Q# users as there are no abstract classes in Q#.

ScottCarda-MS commented 3 years ago

I've now also tried calling a body intrinsic operation outside a unit test, and it does indeed raise the C# exception, which seems like an issue to me. I would think that we would want to communicate it as a Q# exception i.e. "you have a body intrinsic operation defined at such-and-such a location that is not defined in the runtime". I think the unique issue with this kind of error is that it is dependent on the runtime framework's behavior (that of the chosen simulator) but is triggered by something the Q# user does in the Q# source code, namely define an intrinsic for which there is no runtime support. I don't know what the right option here is, whether to surface the information in the context of the runtime (the C# exception) or raise the exception as a Q# error (pointing to the bad operation), but I would lean towards the latter as any Q# user can always trigger that behavior by defining any of an infinite number of unsupported intrinsic operations. Perhaps there is someway we can accommodate both, raising Q# errors with regular Q# users and raising the simulator-specific C# errors when developing simulators, but I wouldn't know how to differentiate between the two scenarios.