Merge feature/azure-quantum-simulator to main #761

cesarzc · 2021-06-30T04:37:46Z

This PR merges the changes made in the feature/azure-quantum-simulator branch to main.

* Create controller project. * Add script to test the QIR controller. * Test QIR controller as part of the build. * Renamed test controller script. * Update test-case file.

…azure-quantum-simulator-20210406 Merge main to feature/azure-quantum-simulator

…ted)

… into alchocro/prototype-controller

…simulator-20210409

QIR controller (part 1)

…n-to-feature-azure-quantum-simulator-20210409

…azure-quantum-simulator-20210409 Merge main to feature/azure-quantum-simulator

DmitryVasilevsky · 2021-07-02T07:06:12Z

src/Qir/Execution/Tools/Driver/QirCppDriver.tt

+<#@ import namespace="System.IO" #>
+<#@ import namespace="Microsoft.Quantum.QsCompiler.BondSchemas.Execution" #>
+//------------------------------------------------------------------------------
+// This code was generated by a tool.


Hard to understand if this file was generated by a tool or if this the source templated used by a tool and generated files will have this comment. Think like a person who sees this codebase for the first time and this is the first file that person sees.

Created a README.md file at the src/Qir/Execution/Tools level that explains the *.tt files and the corresponding generated *.cs files. It also describes a bit more in detail the contents of each subfolder.

DmitryVasilevsky · 2021-07-02T23:16:12Z

src/Qir/Execution/Tools/Executable/QirExecutableGenerator.cs

+                    }
+                }
+
+                var inputFiles = sourceDirectory.GetFiles().Select(fileInfo => fileInfo.FullName).ToArray();


Consider Directory.GetFiles(sourceDirectory.FullName). Shorter and without linq and lambdas.

In this case, Select is used to transform the FileInfo array returned by GetFiles() into a string array. It doesn't really do anything that requires a search pattern.

DmitryVasilevsky · 2021-07-02T23:20:23Z

src/Qir/Execution/Tools/Executable/QirExecutableGenerator.cs

+            }
+        }
+
+        private void CopyDirectoryContents(DirectoryInfo directoryToCopy, DirectoryInfo destinationDirectory)


This uses CopyFileIfNotExists so copies files only if destination file doesn't exist. Suggest reflecting this in the name. Just by the name I'd assume it will overwrite in this situation.

Changed the name to reflect that it only copies the contents if it does not already exist in the target directory,

DmitryVasilevsky · 2021-07-03T00:12:20Z

src/Qir/Execution/Tools/Executable/QuantumExecutableRunner.cs

+            process.EnableRaisingEvents = true;
+            process.Start();
+            var output = await process.StandardOutput.ReadToEndAsync();
+            process.WaitForExit();


Theoretically there's Process.WaitForExitAsync, but the standard output is closed at this time so the process should be done.

I believe in this case it is ok to use WaitForExit instead of WaitForExitAsync since the latter just provides the option to stop waiting using a cancellation token (documentation here). Given that we don't really want the option to stop waiting using a cancellation token, WaitForExit seems like the appropriate option here.

DmitryVasilevsky · 2021-07-03T00:13:37Z

src/Qir/Execution/Tools/QirTools.cs

+
+            if (!executablesDirectory.Exists)
+            {
+                executablesDirectory.Create();


Conditional isn't needed, Create should be a no op if directory exists.

Removed conditional.

DmitryVasilevsky · 2021-07-03T00:17:36Z

src/Qir/Execution/Tools/QirTools.cs

+                executablesDirectory.Create();
+            }
+
+            var tasks = EntryPointOperationLoader.LoadEntryPointOperations(qsharpDll).Select(entryPointOp =>


Not immediately clear if all the builds will run at the same time. Comment?

Added a comment about it.

DmitryVasilevsky · 2021-07-06T17:14:59Z

src/Qir/Execution/Tests.Microsoft.Quantum.Qir.Tools/Util.cs

+        {
+            var filePath = Path.Combine(directory.FullName, fileName);
+            var fileInfo = new FileInfo(filePath);
+            using var fileStream = fileInfo.OpenWrite();


Did you consider File.WriteAllText?

Changed the implementation to use File.WriteAllText.

DmitryVasilevsky · 2021-07-06T17:15:36Z

src/Qir/Execution/Tests.Microsoft.Quantum.Qir.Tools/Util.cs

+        {
+            var filePath = Path.Combine(directory.FullName, fileName);
+            var fileInfo = new FileInfo(filePath);
+            using var fileStream = fileInfo.OpenWrite();


Did you consider File.WriteAllBytes?

Changed the implementation to use File.WriteAllBytes.

DmitryVasilevsky · 2021-07-06T19:45:16Z

src/Qir/Execution/Tests.Microsoft.Quantum.Qir.Tools/Util.cs

+            (int byteA, int byteB) = (0, 0);
+            while ((byteA == byteB) && (byteA != -1))
+            {
+                (byteA, byteB) = (streamA.ReadByte(), streamB.ReadByte());


Probably good enough for us, but see discussion here on performance. https://stackoverflow.com/questions/43289/comparing-two-byte-arrays-in-net

Yes, I believe for our use it should be good enough.

DmitryVasilevsky · 2021-07-06T20:24:29Z

src/Simulation/Core/Submitters/IQuantumMachineExecutionContext.cs

    /// <summary>
    /// Interface to provide configuration details to manage execution.
    /// </summary>
+    [Obsolete("No longer used.")]


I general it would be nice to say what replaces it. And by replacement I don't always mean direct replacement. If we switched to different way of doing things, it's enough to mention this new way of doing things. (Here and other places).

Added more specific information about a possible replacement.

DmitryVasilevsky · 2021-07-06T21:14:30Z

src/Simulation/EntryPointDriver/Azure/Azure.cs

+            if (settings.DryRun)
+            {
+                DisplayError("Dry run is not supported with QIR submission.", null);
+                return 1;


Should we have constants for specific exit codes?

I believe it might be worth doing when we have a larger set of error conditions but for now I believe it is ok to return the same exit code regardless of the specific error.

DmitryVasilevsky · 2021-07-06T21:21:27Z

src/Simulation/EntryPointDriver/Azure/Azure.cs

+        /// </summary>
+        /// <param name="settings">The Azure Quantum submission settings.</param>
+        /// <param name="message">The message.</param>
+        private static void Log(AzureSettings settings, string message)


Suggest reflecting condition in the name, i.e. LogIfVerbose or similar.

Changed name to reflect the condition.

DmitryVasilevsky · 2021-07-06T21:33:02Z

src/Simulation/EntryPointDriver/Azure/AzureSettings.cs

+                return "";
+            }
+
+            return baseUri.Host.Substring(0, baseUri.Host.IndexOf('.'));


Does Host always contain dot?

DmitryVasilevsky · 2021-07-06T21:34:55Z

src/Simulation/EntryPointDriver/Driver.cs

-        private static readonly OptionInfo<string?> JobNameOption = new OptionInfo<string?>(
-            ImmutableList.Create("--job-name"), default, "The name of the submitted job.");
+        private static readonly OptionInfo<string> JobNameOption = new OptionInfo<string>(
+            ImmutableList.Create("--job-name"), "", "The name of the submitted job.");


Doesn't matter much, but String.Empty is considered cleaner.

Changed to string.Empty.

…r-to-main

…ld be public and do not need to.

cesarzc and others added 30 commits April 5, 2021 10:57

Setup controller project (#597)

c7f55a5

* Create controller project. * Add script to test the QIR controller. * Test QIR controller as part of the build. * Renamed test controller script. * Update test-case file.

Merge latest main.

28bad1c

Add missing EndProject.

56469cd

Re-add controller project to Simulation solution.

1dbd8f6

Merge pull request #610 from microsoft/cesarzc/merge-main-to-feature-…

f8d93bb

…azure-quantum-simulator-20210406 Merge main to feature/azure-quantum-simulator

Create initial prototype for controller (with dependencies unimplemen…

0cbc02c

…ted)

Merge remote-tracking branch 'origin/feature/azure-quantum-simulator'…

171825e

… into alchocro/prototype-controller

Remove generated qir

ddf30c8

Remove placeholder tests

dca7fbb

Allow test cases folder not to exist

2c8ca9f

Add default error handling

7558e46

Remove test artifacts from solution

d192415

Create logger and tests

84725d3

Add logs

52cca9f

Fix warning level

3513858

Fix unit tests for linux

74fb929

Fix header

66ddfb0

Add include directory

f2ff7b2

Clean up and fix input

0b19366

Change how directories are passed

ae18d34

Add comment

ef1a2f8

Merge branch 'main' into cesarzc/merge-main-to-feature-azure-quantum-…

f82e3ff

…simulator-20210409

Do not delete output file on error

ce030fa

Merge pull request #620 from microsoft/alchocro/qir-controller-logging

7811d34

QIR controller (part 1)

Merge branch 'feature/azure-quantum-simulator' into cesarzc/merge-mai…

382f3c4

…n-to-feature-azure-quantum-simulator-20210409

Merge pull request #622 from microsoft/cesarzc/merge-main-to-feature-…

e35a7c1

…azure-quantum-simulator-20210409 Merge main to feature/azure-quantum-simulator

Create IQirMachine interface.

22d1b23

Remove ConfigureJob, IQuantumMachine inherits IAzureMachine

7df14c5

Use Stream

15f485b

Add Argument class

4ad0265

DmitryVasilevsky reviewed Jul 2, 2021

View reviewed changes

Address Dmitry's feedback.

59c0436

DmitryVasilevsky reviewed Jul 2, 2021

View reviewed changes

Adding entry point operation loader (#766)

cb251d8

DmitryVasilevsky reviewed Jul 3, 2021

View reviewed changes

Bettina Heim added 2 commits July 2, 2021 17:45

updating package version

cd623e4

missed a package number update

3ab4d8e

DmitryVasilevsky reviewed Jul 6, 2021

View reviewed changes

cesarzc and others added 7 commits July 7, 2021 00:13

Fix namespaces to be backwards compatible.

84fd28f

Rename some interfaces.

c2a0ef5

Make NoOpQirSubmitter internal.

945aab1

Update package number.

094a8b0

Fixed test cases.

0b07469

Merge branch 'main' into cesarzc/merge-feature-azure-quantum-simulato…

15066de

…r-to-main

Remove custom utils from Microsoft.Quantum.Qir.Tools package that wou…

5f4636e

…ld be public and do not need to.

bettinaheim approved these changes Jul 8, 2021

View reviewed changes

cesarzc added 3 commits July 8, 2021 01:26

Addressed more feedback from Dmitry.

ae3b3e1

Rename interface and class for consistency.

b938c44

Renamed folder for consitency.

f64ece9

cesarzc merged commit 9fa45de into main Jul 8, 2021

Merge feature/azure-quantum-simulator to main #761

Merge feature/azure-quantum-simulator to main #761

Uh oh!

Conversation

cesarzc commented Jun 30, 2021

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!

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!

Choose a reason for hiding this comment

Uh oh!

DmitryVasilevsky Jul 3, 2021 • 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!

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!

DmitryVasilevsky Jul 6, 2021 • 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!

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!

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!

9 participants

DmitryVasilevsky Jul 3, 2021 •

edited

Loading

DmitryVasilevsky Jul 6, 2021 •

edited

Loading