-
Notifications
You must be signed in to change notification settings - Fork 90
Making stack trace printout optional #487
Conversation
cgranade
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a reasonable addition to me, thanks for tagging me in.
It would be good to test manually that this works as expected in IQ#, however, as IQ# uses the DisableExceptionPrinting method to control stack traces:
/// <summary>
/// Adds functionality to a given quantum simulator to display
/// diagnostic output and stack traces for exceptions.
/// </summary>
/// <param name="simulator">
/// The simulator to be augmented with stack trace display functionality.
/// </param>
/// <param name="channel">
/// The Jupyter display channel to be used to display stack traces.
/// </param>
/// <returns>
/// The value of <paramref name="simulator" />.
/// </returns>
public static T WithStackTraceDisplay<T>(this T simulator, IChannel channel)
where T: SimulatorBase
{
simulator.DisableExceptionPrinting();
simulator.OnException += (exception, stackTrace) =>
{
channel.Display(new DisplayableException
{
Exception = exception,
StackTrace = stackTrace
});
};
return simulator;
}In particular, the change here may not address @tcNickolas' report in #156, since as per the screenshot in that issue, the stack traces are being generated by the Run method in https://github.com/microsoft/QuantumKatas/blob/eb9a70e150051a87beb04c90a4ef95f073a09b36/utilities/Microsoft.Quantum.Katas/KataMagic.cs#L110:
/// <summary>
/// What this Magic does when triggered. It will:
/// - find the Test to execute based on the given name,
/// - compile the code after found after the name as the user's answer.
/// - run (simulate) the test and report its result.
/// </summary>
public virtual async Task<ExecutionResult> Run(string input, IChannel channel)
{
channel = channel.WithNewLines();
// Expect exactly two arguments, the name of the Kata and the user's answer (code).
var args = input?.Split(new char[] { ' ', '\n', '\t' }, 2);
if (args == null || args.Length != 2)
{
channel.Stderr("Invalid parameters. Usage: `%kata Test \"Q# operation\"`");
return ExecuteStatus.Error.ToExecutionResult();
}
var name = args[0];
var code = args[1];
var test = FindTest(name);
if (test == null)
{
channel.Stderr($"Invalid test name: {name}");
return ExecuteStatus.Error.ToExecutionResult();
}
var userAnswer = Compile(code, channel);
if (userAnswer == null) { return ExecuteStatus.Error.ToExecutionResult(); }
return Simulate(test, userAnswer, channel)
? "Success!".ToExecutionResult()
: ExecuteStatus.Error.ToExecutionResult();
}Modifying that magic command to use redirect stack traces through DisplayableExceptionEncoders.cs and then making those encoders more configurable (likely using IQ#'s IConfigurationSource functionality) would allow for fine-tuning how much stack trace information is shown from the %kata magic command.
|
Just to clarify (I need to post the offline discussion to that issue for the most up-to-date status): I'm happy with the current behavior in Q# Jupyter notebooks, it's the stack trace in the Katas executed as Q# standalone projects that is excessive for me. |
Ah, got it; I had misunderstood from the other issue, then. In that case, I'm fine with this PR as-is, other than that an API comment would help prevent confusion about when the new property applies. |
This change allows control over stack trace printout. Added EnableStackTracePrinting property on SimulatorBase. Set to true (default) to enable printout, set to false to disable printout. Only stack trace printout is disabled, exception message is still printed.
This partially addresses issue #156