Begin simplifying and updating the Get-GateCount sample #202
Conversation
There was a problem hiding this comment.
In general it looks good. We need to be consistent on properties vs fields and their naming (they should all be capitalized.
I like how the configuration have pulled into their own structs. I would take it even further and add EntryOperation to avoid some code duplication in RunGateCount.
As you pointed out this morning. Marking this as Approved although it is more like Approved With Suggestions.
| public double CNOTCount; | ||
| public double NormMultipler; | ||
| public long ElapsedMilliseconds; | ||
| public Dictionary<string, string> TraceSimulationStats; |
There was a problem hiding this comment.
Should we keep these? I don't see them used or printed.
There was a problem hiding this comment.
They are in principle accessible via the PowerShell interface, but they're not otherwise used, no.
|
|
||
| var gateCountResults = new GateCountResults(); | ||
| #region Trace Simulator on Trotter step | ||
| if (config.hamiltonianSimulationAlgorithm == HamiltonianSimulationAlgorithm.ProductFormula) |
There was a problem hiding this comment.
Each of these are mostly copy/paste... I would take the Configuration concept and go a step further and move the which Q# operation to run as part of the configuration and a Name property, so you don't have to have the code duplication here.
There was a problem hiding this comment.
Agreed. I actually tried to do that, but ran into a couple slight issues in that the GetMetric method of the trace simulator takes a type argument for the operation in question, and in that the optimized qubitization operation takes an additional parameter for the target error. I'd prefer if possible to defer this improvement to the next PR on this sample if that's OK.
| /// <summary> | ||
| /// Configuration data for Hamiltonian simulation algorithm | ||
| /// </summary> | ||
| public struct HamiltonianSimulationConfig |
There was a problem hiding this comment.
Instead of a HamiltonianSimulationConfig class with different config properties depending on the type of algorithm, I would create an IHamiltonianSimulationConfig interface with a couple of methods needed to run the simulation, for example HamiltonianSimulationAlgorithm and EntryOperation, and then have the ProductFormulaConfig and QubitizationConfig implement this interface.
This would also address the duplicated code in RunGateCount.
…t/Quantum into cgranade/simplify-get-gatecount
|
Merging in now, with future work tracked in #203. Thanks! |
* Begin simplifying and updating the Get-GateCount sample (#202) * Ignore gate count outputs. * Begin simplifying gate count sample. * More work on simplifying the gate count sample, updating to beta package. * Removed a few levels of nested types. * First → Single, fixed plotting issues. * Added some blank lines for readability. * Changed fields to properties, applied uppercase naming. * Fixed some comments in powershell integration. * Moving to script-based build. * Moving to script-based build. * Moving to 0.8.1906.2007-beta * Validation of IntegralData only supported in Windows * Adding step to publish test results. * Update samples for 0.8 beta. (#206) * Beginning work on converting samples to 0.8. * Enabled using devcontainer to lock Quantum Development Kit version. * Updated several samples to 0.8 beta. * Updated devcontainer to 0.8 beta. * Updated several more samples to 0.8. * Updated a few more projects. * Adapting development container for working with chemistry, python. * Add QInfer to docker, put metadata back. * Added mpl requirements to Dockerfile. * Updated tomography sample to 0.8. * Fixed warning in DatabaseSearch. * Fixed typo. * Update .devcontainer/README.md Co-Authored-By: Andres Paz <anpaz@microsoft.com> * Fixed spelling in F# solution. * Step 1 of renaming F# driver. * Step 2 of renaming F# driver. * Updating qdk to latest version
This PR begins to simplify the gate count sample by splitting the one Program.cs into several files that are each easier to read, making outputting CSVs optional, and by consolidating common logic into a few extension methods.