Cleanup in M.Q.Preparation #315

msoeken · 2020-08-25T14:53:30Z

This is the first PR into the M.Q.Preparation feature branch, starting with some cleanup of the existing code.

guenp

Just a few comments.

Chemistry/src/Runtime/JordanWigner/JordanWignerOptimizedBlockEncoding.qs

Standard/src/Preparation/Arbitrary.qs

guenp · 2020-08-31T21:24:59Z

Standard/src/Preparation/QuantumROM.qs

-    // |coefficient[i] * oneNorm - discretizedCoefficient[i] * discreizedOneNorm| * nCoeffs <= 2^{1-bitsPrecision}.
-    function _QuantumROMDiscretization(bitsPrecision: Int, coefficients: Double[]) : (Double, Int[], Int[]) {
+    // |coefficient[i] * oneNorm - discretizedCoefficient[i] * discretizedOneNorm| * nCoeffs <= 2^{1-bitsPrecision}.
+    function _QuantumROMDiscretization(bitsPrecision: Int, coefficients: Double[])


Perhaps it would also be good to use internal function here for consistency

Unfortunately this function is tested in a separate test project, so it cannot be internal (for now). So I leave the _ such that it does not end up in docs.

Should we make a qsharp-compiler feature request for internalsvisibleto?

in this case would it be an idea to create the internal function and make _QuantumROMDiscretization a wrapper to it with a deprecation note, and create a task to refactor said test project?

I would leave this for now until we have a solution. I agree with @cgrande to open a feature request for this. An alternative solution would be to have a non-internal function with an attribute such as @UsedForTesting indicating that no docs should be generated.

msoeken · 2020-09-01T07:23:40Z

Thanks for your comments @guenp.

cgranade

I think these look like some really good improvements, thanks for doing this, @msoeken!

cgranade · 2020-08-27T15:43:21Z

Standard/src/Preparation/Arbitrary.qs

    // NB: This is currently not marked as internal, as the QML library
    //     currently uses this function. Please see the relevant GitHub issue
    //     at https://github.com/microsoft/QuantumLibraries/issues/239.
-    function _CompileApproximateArbitraryStatePreparation(


As per the comment above, this function is currently called from the QML library; it'd be good to double-check that the QML library works with this function being internal.

Chemistry/src/Runtime/JordanWigner/JordanWignerOptimizedBlockEncoding.qs

Standard/src/Preparation/Mixed.qs

Standard/src/Preparation/Arbitrary.qs

cgranade · 2020-09-01T15:39:46Z

Standard/src/Preparation/QuantumROM.qs

-    // |coefficient[i] * oneNorm - discretizedCoefficient[i] * discreizedOneNorm| * nCoeffs <= 2^{1-bitsPrecision}.
-    function _QuantumROMDiscretization(bitsPrecision: Int, coefficients: Double[]) : (Double, Int[], Int[]) {
+    // |coefficient[i] * oneNorm - discretizedCoefficient[i] * discretizedOneNorm| * nCoeffs <= 2^{1-bitsPrecision}.
+    function _QuantumROMDiscretization(bitsPrecision: Int, coefficients: Double[])


Should we make a qsharp-compiler feature request for internalsvisibleto?

Standard/src/Preparation/UniformSuperposition.qs

Standard/src/Preparation/Arbitrary.qs

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

…oft/QuantumLibraries into msoeken/state-preparation-cleanup

msoeken · 2020-09-02T11:06:00Z

Thanks for your comments @cgranade, @guenp and @SamarSha. I re-request your review @cgranade, since I added new public API to M.Q.Canon (IsRangeEmpty) after your comment.

cgranade

Approving without API review since this targets a feature branch rather than master; let's make sure to add IsRangeEmpty to the schedule before merging that feature to master. Thanks!

cgranade · 2020-09-02T14:53:44Z

Standard/src/Canon/Range.qs

+
+    /// # Summary
+    /// Returns true if and only if input range is empty.
+    ///


It will be pretty basic in this case, but could we add an /// # Input and /// # Output block here?

* Cleanup in M.Q.Preparation (#315) * Cleanup QuantumROM. * Cleanup uniform superposition. * Cleanup quantum ROM. * Clean-up arbitrary state preparation. * Cleanup remaining files. * Fix bug in QuantumROM. * Fix error. * Revert name due to #239. * Apply suggestions from code review Co-authored-by: Chris Granade <chgranad@microsoft.com> * Addressing reviewer comments. * Docs. Co-authored-by: Mathias Soeken <masoeken@microsoft.com> Co-authored-by: Chris Granade <chgranad@microsoft.com> * Fix namespace. * Extend PurifiedMixedState for additional data preparation (#322) * Squashed commit of the following: commit 9841567 Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 10:41:17 2020 +0200 Fix bug in QuantumROM. commit 9726ff2 Merge: 58d428e 0b02a6a Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 09:49:43 2020 +0200 Merge branch 'master' into msoeken/state-preparation-cleanup commit 58d428e Author: Mathias Soeken <masoeken@microsoft.com> Date: Thu Aug 27 11:59:12 2020 +0200 Cleanup remaining files. commit b35222a Author: Mathias Soeken <masoeken@microsoft.com> Date: Thu Aug 27 11:54:33 2020 +0200 Clean-up arbitrary state preparation. commit e8076f7 Author: Mathias Soeken <masoeken@microsoft.com> Date: Wed Aug 26 09:31:17 2020 +0200 Cleanup quantum ROM. commit 860c0ab Author: Mathias Soeken <masoeken@microsoft.com> Date: Wed Aug 26 09:21:21 2020 +0200 Cleanup uniform superposition. commit b94ec8c Author: Mathias Soeken <masoeken@microsoft.com> Date: Tue Aug 25 15:56:26 2020 +0200 Cleanup QuantumROM. * Remove duplicate code and reduce T-count. * Cleanup. * Squashed commit of the following: commit cb77faa Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 16:46:12 2020 +0200 Fix error. commit 9841567 Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 10:41:17 2020 +0200 Fix bug in QuantumROM. commit 9726ff2 Merge: 58d428e 0b02a6a Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 09:49:43 2020 +0200 Merge branch 'master' into msoeken/state-preparation-cleanup commit 58d428e Author: Mathias Soeken <masoeken@microsoft.com> Date: Thu Aug 27 11:59:12 2020 +0200 Cleanup remaining files. commit b35222a Author: Mathias Soeken <masoeken@microsoft.com> Date: Thu Aug 27 11:54:33 2020 +0200 Clean-up arbitrary state preparation. commit e8076f7 Author: Mathias Soeken <masoeken@microsoft.com> Date: Wed Aug 26 09:31:17 2020 +0200 Cleanup quantum ROM. commit 860c0ab Author: Mathias Soeken <masoeken@microsoft.com> Date: Wed Aug 26 09:21:21 2020 +0200 Cleanup uniform superposition. commit b94ec8c Author: Mathias Soeken <masoeken@microsoft.com> Date: Tue Aug 25 15:56:26 2020 +0200 Cleanup QuantumROM. * Squashed commit of the following: commit cde7c0c Author: Mathias Soeken <masoeken@microsoft.com> Date: Mon Aug 31 16:34:40 2020 +0200 Unzipped function. * Preparing function and tests for QuantumROM with signed coefficients. * Further unification. * Ineffecient way of creating sign bit. * Embed sign computation in multiplex. * Picking changes from #212. * Changes from #212. * Generalize. * Generalize data register. * Unifying signatures. * Use UDT for PurifiedState. * Docs and cleanup. * Incoprate API changes. * Fix bug on Mac. * Make public because of #239. * Add deprecated functions. Co-authored-by: Mathias Soeken <masoeken@microsoft.com> Co-authored-by: Chris Granade <chgranad@microsoft.com> * Implement last changes from #344 (#375) * Implemented changes from #344 review. * Ported signed quantum ROM test to use WithData. * Made AssertSignedProbInt internal until #337. * Apply suggestions from code review Co-authored-by: Mathias Soeken <mathias.soeken@outlook.com> Co-authored-by: Mathias Soeken <mathias.soeken@outlook.com> * Apply suggestions from code review Co-authored-by: Guen P <guenp@microsoft.com> * Apply suggestions from code review Co-authored-by: Mathias Soeken <mathias.soeken@outlook.com> Co-authored-by: Mathias Soeken <mathias.soeken@outlook.com> Co-authored-by: Mathias Soeken <masoeken@microsoft.com> Co-authored-by: Guen P <guenp@microsoft.com>

Cleanup QuantumROM.

b94ec8c

msoeken changed the base branch from master to feature/state-preparation August 25, 2020 14:53

msoeken added 4 commits August 26, 2020 09:21

Cleanup uniform superposition.

860c0ab

Cleanup quantum ROM.

e8076f7

Clean-up arbitrary state preparation.

b35222a

Cleanup remaining files.

58d428e

msoeken requested review from anpaz and cgranade and removed request for anpaz August 27, 2020 09:59

msoeken marked this pull request as ready for review August 27, 2020 10:00

msoeken added 2 commits August 31, 2020 09:49

Merge branch 'master' into msoeken/state-preparation-cleanup

9726ff2

Fix bug in QuantumROM.

9841567

msoeken requested a review from guenp August 31, 2020 14:36

msoeken added 2 commits August 31, 2020 16:46

Fix error.

cb77faa

Revert name due to #239.

540fbe8

guenp reviewed Aug 31, 2020

View reviewed changes

cgranade approved these changes Sep 1, 2020

View reviewed changes

msoeken and others added 3 commits September 2, 2020 13:03

Apply suggestions from code review

d065ca6

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

Addressing reviewer comments.

f66d5ba

Merge branch 'msoeken/state-preparation-cleanup' of github.com:micros…

3941172

…oft/QuantumLibraries into msoeken/state-preparation-cleanup

msoeken requested a review from cgranade September 2, 2020 11:06

cgranade approved these changes Sep 2, 2020

View reviewed changes

Docs.

dd51f63

msoeken merged commit 3ab32ab into feature/state-preparation Sep 3, 2020

msoeken deleted the msoeken/state-preparation-cleanup branch September 3, 2020 09:53

Cleanup in M.Q.Preparation #315

Cleanup in M.Q.Preparation #315

Uh oh!

Conversation

msoeken commented Aug 25, 2020

Uh oh!

guenp left a comment

Choose a reason for hiding this comment

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!

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!

msoeken commented Sep 1, 2020

Uh oh!

cgranade left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

msoeken commented Sep 2, 2020

Uh oh!

cgranade left a comment

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