Durr hoyer library min and max #1936

mertall · 2024-09-25T23:29:26Z

Unit Tests worked standalone with 1000 shots. Above 50% accuracy. 💪🏾 (When running locally in my own GitHub Repo)

Can't seem to get DrawRandomInt to work with python ./build.py. I can't find any subitutes other than a QuantumRandomNumberGenerator, but that is not feasible when I read the docs on the implementation of the Operation in Q# docs. I was thinking maybe allowing the random int to come from outside of the Algorithm as an input, to be found in a classical computer via a python script run by a hypothetical user.

Let me know what you all think. @sezna @billti @Manvi-Agrawal

Issue #1928

…il fix was implemented, takes advantage of optimized slice pattern matching native to rust

…andomInt(), reference microsoft#1928

mertall · 2024-09-25T23:34:59Z

@microsoft-github-policy-service agree

library/std/src/Std/DurrHoyerLibrary.qs

Co-authored-by: Manvi-Agrawal <[email protected]>

library/std/src/Std/DurrHoyerLibrary.qs

Manvi-Agrawal · 2024-09-26T01:00:34Z

library/src/tests/resources/src/durrhoyerlibrary.qs

Some of the comments from previous file regarding imports apply here.

Also, could you please extract common logic from RunDHMinUT and RunDHMaxUT?

library/src/tests/resources/src/durrhoyerlibrary.qs

Manvi-Agrawal · 2024-09-26T01:03:34Z

language_service/src/completion.rs

@@ -247,7 +247,7 @@ fn get_indent(compilation: &Compilation, package_offset: u32) -> String {
        .try_into()
        .expect("offset can't be converted to uszie");
    let before_offset = &source.contents[..source_offset];
-    let mut indent = match before_offset.rfind(|c| c == '{' || c == '\n') {
+    let mut indent = match before_offset.rfind(&['{', '\n'][..]) {


Do we really need to change this? IMO this PR should focus only on implementing DH library.

I saw your commit message saying that build.py doesnt work for you without this change. I will leave this decision to @sezna.

If there is a way to skip over the error when I run the build.py I am happy to change it. I agree it is out of place, and will be hard to log trace down the line while it is included in this PR. maybe we can create a ticket to address this and I can push a PR for that?

Manvi-Agrawal · 2024-09-26T01:06:58Z

@mertall , Mostly, looks good. Left a couple of nits regarding import and code reuse. Overall you have done a good job writing separating code into functions and using meaningful variable names. Could you please look further into some more places if the code can be reused or a library function can be used. I pointed at a couple of places.

library/std/src/Std/DurrHoyerLibrary.qs

… folder

…for max test case

library/std/src/Std/DurrHoyerLibrary.qs

…ects the expectedMaxaindices variable

library/src/tests/resources/src/durrhoyerlibrary.qs

Manvi-Agrawal · 2024-09-26T02:00:02Z

library/src/tests/resources/src/durrhoyerlibrary.qs

+            let double : Double = IntAsDouble(maxValue + 1);
+            let log : Double = Log(double) / Log(2.0);
+            let nQubits = Ceiling(log);
+            let testLists : Int[][] = [


Some comments on the tests:

What happens if some/all of the values are negative in a testList?

What happens if 0 is passed as input in a testList?

Can we please rearrange the values so that expectedMaxIndex is not 0 in all the test cases?

for negatives in ConvertToBinary:

if (value < 0 or value > maxVal) {
fail $"Value {value} cannot be represented with {length} bits.";
}

ConvertToBinary will fail if there are negatives

Added unit test for 0 in input list

… reopening closed suggestions

mertall · 2024-09-26T03:11:04Z

moved too fast on requested changes, lost 50% accuracy while trying to abstract...

I get better debugging as a local Q# project. When working within the Q# repo I forked, I do not get the same debugging... Wondering if you had any suggestions to improve dev experience while in forked repo. Made an assumption earlier the debugging is same and went flying with changes. Once I moved it over to my Q# project to update that repo, I saw so many squiggly red lines :o ... :') .

for now solution will be to make changes in my local Q# project as debugging is much easier. Will get back to this maybe tomorrow, more likely Friday. Thank you for the review!

Manvi-Agrawal · 2024-09-26T04:15:34Z

I get better debugging as a local Q# project. When working within the Q# repo I forked, I do not get the same debugging... Wondering if you had any suggestions to improve dev experience while in forked repo. Made an assumption earlier the debugging is same and went flying with changes. Once I moved it over to my Q# project to update that repo, I saw so many squiggly red lines :o ... :') .

If you could elaborate on what is slowing you down it would help us understand this better. I am curious are you running build.py everytime to run the tests? That can be a major deal-breaker. I have worked on compiler module a bit to implement the lint rules, and running rust unit tests was very fast. I guess you are need to ensure that tests in library/src/tests/durrhoyerlibrary.rs pass. After a first successful build with build.py, you should just run specific Rust unit test(s). First unit test run might be a bit slow, and then it gets better.

You can click on "Run Test" option that comes up in the test file to run a particular test. You should get this option if your repo is properly setup. It also shows command run under the hood C:\Users\manvi\.cargo\bin\cargo.exe test --package library --lib -- tests::arrays::check_all --exact --show-output in this case.

I was able to run all of the tests in library\src\tests\arrays.rs by running the command on powershell that vscode by omitting the test name. I ran C:\Users\manvi\.cargo\bin\cargo.exe test --package library --lib -- tests::arrays --exact --show-output locally in powershell on windows. Based on this observation, I think this command might help you <cargo.exe_path> test --package library --lib -- tests::durrhoyerlibrary --exact --show-output.

mertall · 2024-09-26T13:53:11Z

<cargo.exe_path> test --package library --lib -- tests::durrhoyerlibrary --exact --show-output

this should help me. I was running the build and made an assumption the tests I created were included during that build. Thank you!

mertall · 2024-09-26T14:15:33Z

my rust test file is improperly created. vs code is not highlighting any code on that file. I am inexperienced in rust so could use help in adjusting that script to meet whatever requirements I am missing. In mean time will make changes on my GitHub repo and copy and paste changes as they work. Thank you of your patience

… find the DurrHoyerAlgorithm

mertall · 2024-09-26T16:35:09Z

Testing suite is working, and the test cases are getting hit, but they are not passing due to this error:

error: Frontend(Error(Resolve(GlobImportNamespaceNotFound("DurrHoyerLibrary", Span { lo: 283, hi: 303 })))) })
...
error: Frontend(Error(Resolve(NotFound("DurrHoyerAlgorithm", Span { lo: 1400, hi: 1418 }))))

sezna · 2024-09-26T16:43:17Z

Hey @mertall -- this looks like awesome work!

I'm not sure this is a good fit for the standard library, though. The standard library is for things that have gone through a process of stabilization, with an API that we're willing to commit to forever. I think this algorithm would be best published as a Q# package (library). If it gets wide adoption, is seen as a "general utility", and we're happy with the API, then we can consider adding it to the standard library. Until then, I would suggest maintaining it as a library that you own.

We have a document here describing how to publish libraries for Q#: our repo wiki and another document on learn.microsoft.com.

After you create your library, you can add it to our registry. Once #1932 goes in, you can put up a PR that adds your library as a list of known libraries.

I hope I'm not taking any wind out of your sails here -- our standard library is not meant to be a good example of a library, as it requires a lot of special casing for testing etc. The other libraries, like fixed_point, are true libraries. I'd recommend using those as a template. You'll notice their tests are written in Q# and not Rust.

mertall · 2024-09-26T17:16:23Z

Thank you for the feedback @sezna. To clarify steps moving forward:

Maintain project in my own repo here
Set this qsharp repo as an external dependency for my above project
once that issue is in, PR for my above project.

No worries on taking any wind out of my sails. I can't say I was not excited to contribute to Q#! When the time is right it shall happen. Thank you for the support so far!

sezna · 2024-09-26T18:27:35Z

No worries on taking any wind out of my sails. I can't say I was not excited to contribute to Q#! When the time is right it shall happen. Thank you for the support so far!

This is absolutely a contribution to Q#, we are really excited about third party libraries and would love to help you get that set up! And there's plenty of opportunity to contribute here as well 😄

mertall added 2 commits September 25, 2024 18:25

refactors searching pattern as python build.py was throwing error unt…

c320532

…il fix was implemented, takes advantage of optimized slice pattern matching native to rust

initial version of min max of durr hoyer algorithm, issues with DrawR…

3d7722f

…andomInt(), reference microsoft#1928

mertall requested review from cesarzc, DmitryVasilevsky, swernli, billti, idavis, minestarks and ScottCarda-MS as code owners September 25, 2024 23:29

adds finding max testing

6b339de