Simplify gradient estimation and classifier structure generation #200
Conversation
* Clarify the restriction on the number of bits for IntAsBoolArray This should fix #166 by providing a more specific error message. * Update Standard/src/Convert/Convert.qs Co-Authored-By: Chris Granade <chgranad@microsoft.com> * Allow to have bits = 0 Looks like our tests assume that number = 0 with bits = 0 is a valid scenario; updating the change to account for that
* Two AND gate implementations. * Added test case. * Formatting. * Code formatting. * Update Standard/src/Canon/And.qs Co-Authored-By: Chris Granade <chgranad@microsoft.com> * Assertion for 0-target. * Added DOI to references. * Named application for CCNOTop. * Rename operations. * Add Test attribute. * Add links to arXiv. * Rename operations. * Better assertion for 0-target. * Fix bug in LowDepthAnd. * Docs. * Doc string convention. * Controlled variant for `ApplyAnd`. * Controlled AndLowDepth. * Adjoint Controlled LowDepthAnd. * References. * Simplify code. * Apply suggestions from code review Co-Authored-By: Chris Granade <chgranad@microsoft.com> * Integrate comment.
There appears to be no function IncrementByIntegerPhaseLE, and I guess it is covered by ApplyLEOperationOnPhaseLE. Co-authored-by: Chris Granade <cgranade@gmail.com>
* First work on Hadamard and SWAP test operations. * (c) header and typo fix. * Fixed typo with placement of phase shift. * Put public operations above private. * Added tests for new operations. * Added API documentation comments. * Newline at end of file.
* Began simplifying AA interface. * Expose traditional AA as new public operation. * Removed rest of "AmpAmp" prefix. * Resolve deprecation warning.
|
As an update, moving the circuit structure generation logic out of the C# library into Q# means that the C# part is no longer actively used, such that I've gone on and consolidated everything back into a single-project layout for the QML library. |
There was a problem hiding this comment.
I was wondering about the practice of creating empty arrays and then populating them by iterative concatenation.
For example
set complexCoefficients += [ComplexPolar(coef, ang)];
where the array complexCoefficients is inialized as empty. A few more instances like that across the code.
Doesn't this create a steady stream of temporaries that then need to be garbage-collected?
What is the wisdom of doing this in a situation when the size of the array is known upfront?
|
Thanks for the review, @alexeib2!
This is a private operation, but all the same, good to rename to be consistent; I'll go on and do that.
Thankfully, this isn't the case. Both approaches are indeed 𝑂(𝑛), as can be confirmed using BenchmarkDotNet:
My motivation here has been to prioritize maintainability and readability in implementations, but I'm also happy to switch to using (As a quick tip, it makes it much easier to find and respond to comments if you attach them to particular lines in the source code. Thanks!) |
|
@alexib2: Thanks again for your review. I've addressed your feedback in the most recent commit, if you could please review and mark as approved then we can get this in. Thanks! |
This PR includes recent changes from master into #176 and uses these changes to simplify gradient estimation. This PR also moves classifier structure specification from C# methods to Q# functions, making them easier to use from other .NET languages or from Python.
I will shortly make a new PR on the samples repo as well with the needed matching changes to the samples.