Auto-generate entry point for Azure job submission #156

rmshaffer · 2020-06-03T21:46:59Z

This PR contains the code to auto-generate an @EntryPoint() operation for Azure Quantum job submission from IQ#. This operation then gets compiled into a separate executable assembly that references the Workspace and Snippets assemblies.

The key places to start reviewing would be:

new function BuildEntryPoint() in src/Core/Compiler/CompilerService.cs
new class EntryPointGenerator in src/AzureClient/EntryPoint/EntryPointGenerator.cs
new class EntryPoint in src/AzureClient/EntryPoint/EntryPoint.cs
consuming function SubmitOrExecuteJobAsync() in src/AzureClient/AzureClient.cs

Fixes AB#13591, fixes AB#15418.

Co-authored-by: Chris Granade <chgranad@microsoft.com>

src/AzureClient/AzureClient.cs

src/AzureClient/EntryPoint/EntryPointOperationResolver.cs

src/AzureClient/EntryPoint/EntryPointGenerator.cs

src/AzureClient/EntryPoint/IEntryPointGenerator.cs

bamarsha · 2020-06-10T20:32:16Z

src/AzureClient/Magic/SubmitMagic.cs

+            var decodedParameters = new Dictionary<string, string>();
+            foreach (var key in inputParameters.Keys)
+            {
+                decodedParameters[key] = inputParameters.DecodeParameter<string>(key);
+            }


Suggested change

var decodedParameters = new Dictionary<string, string>();

foreach (var key in inputParameters.Keys)

{

decodedParameters[key] = inputParameters.DecodeParameter<string>(key);

}

var decodedParameters = inputParameters.ToDictionary(

item => inputParameters.DecodeParameter<string>(item.Key),

item => item.Value);

This is an optional suggestion. :) Just wanted to throw the LINQ version out there.

This is great, I always prefer LINQ, thanks for pointing this out! I'm going to apply this change in one of my other PRs since I've already started refactoring this a bit on another branch.

bamarsha · 2020-06-10T21:09:26Z

src/Core/Compiler/CompilerService.cs

+                    {{
+                        return {operation.Header.QualifiedName}{argumentTuple};
+                    }}
+                }}";


Instead of compiling this string, is it possible to attach the EntryPoint attribute to the callable in the compiled Q# syntax tree, maybe as a rewrite step? Not sure if there are any issues with that way.

I had some discussions about this with @bettinaheim and @anpaz-msft. The reason I ended up doing it this way is that we want to ignore any @EntryPoint() attributes that happen to appear inside snippets (code from notebook cells), and we are doing this by compiling the snippets as a library and suppressing the resulting warning.

If we want to attach the @EntryPoint() attribute directly to the operation in the snippet code and compile the snippets as an executable, then we'd have to also go remove the attribute from any other snippet operations where it might exist. I think this is certainly possible by manipulating the syntax tree, as you suggest, and maybe we can change to go that route in the future if needed, but this seemed like the simplest solution at the moment.

One potential benefit of this approach is that in the future it may be possible to reference other Q# projects from IQ#, including Q# library projects. This approach would also give a way to submit those operations to Azure Quantum without having to manually create a wrapper operation in the notebook.

Seems reasonable, I agree that this is probably simpler. Thanks for explaining!

One potential benefit of this approach is that in the future it may be possible to reference other Q# projects from IQ#, including Q# library projects. This approach would also give a way to submit those operations to Azure Quantum without having to manually create a wrapper operation in the notebook.

I haven't tested it, but you could probably still attach the entry point attribute to callables in a reference. I think they all get added to the same syntax tree.

cgranade

Just a few minor suggestions, thanks!

cgranade · 2020-06-10T23:00:32Z

src/AzureClient/AzureClient.cs

+            var timeoutInSeconds = 30;
+            channel.Stdout($"Waiting up to {timeoutInSeconds} seconds for Azure Quantum job to complete...");
+
+            using (var cts = new System.Threading.CancellationTokenSource(TimeSpan.FromSeconds(30)))


Should this timeout be configurable? That may be doable with the %config magic.

Yes! In fact I have made it configurable in #161.

cgranade · 2020-06-10T23:00:43Z

src/AzureClient/AzureClient.cs

+                {
+                    // TODO: Once jupyter-core supports interrupt requests (https://github.com/microsoft/jupyter-core/issues/55),
+                    //       handle Jupyter kernel interrupt here and break out of this loop 
+                    var pollingIntervalInSeconds = 5;


Similarly here.

Same, this is configurable in #161. Reading my mind...

src/AzureClient/EntryPoint/EntryPoint.cs

src/AzureClient/EntryPoint/EntryPointGenerator.cs

src/AzureClient/EntryPoint/IEntryPointGenerator.cs

src/Tests/SNIPPETS.cs

Co-authored-by: Chris Granade <chgranad@microsoft.com>

rmshaffer · 2020-06-11T19:16:45Z

Thanks @ricardo-espinoza @SamarSha @cgranade for the detailed review and feedback! I think I've responded to all of the comments, but please let me know if I've missed something. When you have a chance, please let me know whether you have any additional concerns/feedback, or whether you're comfortable approving these changes.

ricardo-espinoza · 2020-06-06T01:00:39Z

src/AzureClient/EntryPoint/EntryPoint.cs

+
+            // Find and invoke the method on IQuantumMachine that is declared as:
+            // Task<IQuantumMachineJob> SubmitAsync<TInput, TOutput>(EntryPointInfo<TInput, TOutput> info, TInput input)
+            var submitMethod = typeof(IQuantumMachine)


Following up offline with @rmshaffer on this. Requiring reflection to find the method to call might be needed, but we should discuss ways to avoid it. An option could be to find an abstraction that could encapsulate input/output in some sort of composition type so the IQuantumMachine method could be generic and could be found without searching through the assembly.

Closing the loop here. No objections with keeping it as it is, but it's something we could discuss in the future in terms of the design of the interface itself.

Agreed, thanks @ricardo-espinoza! One minor note for additional context is that the IQuantumMachine interface is defined in the qsharp-runtime repo and is also used by the Q# standalone executable framework, which uses generated code to call these methods and so doesn't require reflection. The strong type safety is nice in that case.

rmshaffer and others added 30 commits May 28, 2020 18:03

Updates to IQ# syntax highlighting

89282df

Validate targets and load provider packages

f53eb9c

Update Python interface for Azure commands

75a8e35

Merge branch 'feature/azure-client' into rmshaffer/azure-targets

1787ea4

Simplify AzureExecutionTarget class

52098e6

Generate EntryPoint and compile into new assembly

53fd8e9

Changes for JobNotCompleted case

a33a2c9

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

e47c9df

Refactor entry point code into new classes

4449040

Use correct input and output types

6f3a6fe

Simplify property syntax

d47fddd

Co-authored-by: Chris Granade <chgranad@microsoft.com>

Add simple tests for AzureExecutionTarget class

5215155

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

eeb4b11

Recompile everything for specified execution target

5e62413

Add tests and error handling

26a79ea

Improve variable names

69a4fe6

Add status message while loading provider package

c63a006

Merge branch 'feature/azure-client' into rmshaffer/azure-targets

1c7d83f

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

cbdd00e

Merge branch 'feature/azure-client' into rmshaffer/azure-targets

944cec0

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

12c7e12

Merge branch 'feature/azure-client' into rmshaffer/azure-targets

b8e680b

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

126f43b

Add documentation to AzureExecutionTarget.GetProvider

43c07ae

Merge branch 'feature/azure-client' into rmshaffer/azure-targets

2552632

Merge branch 'rmshaffer/azure-targets' into rmshaffer/azure-entrypoint

4bf71db

Extend tests to call EntryPoint.SubmitAsync

52c481d

Wait for job completion on %azure.execute

a470662

Update comment

354d826

Add timeout for %azure.execute

9041701

rmshaffer added 3 commits June 5, 2020 10:51

Minor fixes in AzureClient

0300ecd

Minor formatting and comment tweaks

8ce9c63

Style improvements in test code

e3e7bcc

rmshaffer marked this pull request as ready for review June 5, 2020 15:04

rmshaffer requested review from bamarsha, bettinaheim and ricardo-espinoza June 5, 2020 15:05

rmshaffer added 3 commits June 5, 2020 13:32

More consistent handling of job objects

f807723

Consistent error handling for IWorkspace calls

1409213

Update to latest QDK released version

78196ff

rmshaffer requested a review from anpaz June 5, 2020 20:24

ricardo-espinoza reviewed Jun 6, 2020

View reviewed changes

src/AzureClient/AzureClient.cs Show resolved Hide resolved

ricardo-espinoza reviewed Jun 6, 2020

View reviewed changes

src/AzureClient/EntryPoint/EntryPointOperationResolver.cs Show resolved Hide resolved

Base automatically changed from rmshaffer/azure-targets to feature/azure-client June 8, 2020 14:02

rmshaffer added 2 commits June 8, 2020 10:05

Merge branch 'feature/azure-client' into rmshaffer/azure-entrypoint

fce176e

Merge branch 'feature/azure-client' into rmshaffer/azure-entrypoint

0164134

This was referenced Jun 8, 2020

Add simple display encoders for CloudJob, TargetStatus, and Histogram #160

Merged

Support several parameters for IQ# Azure Quantum job execution #161

Merged

bamarsha reviewed Jun 10, 2020

View reviewed changes

cgranade reviewed Jun 10, 2020

View reviewed changes

Ryan Shaffer and others added 3 commits June 10, 2020 19:52

Use switch syntax for entryPointInput

9b3b74b

Co-authored-by: Chris Granade <chgranad@microsoft.com>

Addressing PR feedback and compiler warnings

0429635

ricardo-espinoza approved these changes Jun 12, 2020

View reviewed changes

rmshaffer merged commit 1c17687 into feature/azure-client Jun 12, 2020

rmshaffer deleted the rmshaffer/azure-entrypoint branch June 12, 2020 19:31

rmshaffer mentioned this pull request Jun 23, 2020

Azure Quantum integration for IQ# #177

Merged

Auto-generate entry point for Azure job submission #156

Auto-generate entry point for Azure job submission #156

Uh oh!

Conversation

rmshaffer commented Jun 3, 2020 • edited by azure-boards bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bamarsha Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgranade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rmshaffer commented Jun 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rmshaffer commented Jun 3, 2020 •

edited by azure-boards bot

Loading

bamarsha Jun 10, 2020 •

edited

Loading