Skip to content

updated hhl#1314

Merged
roie-d-classiq merged 3 commits intomainfrom
hhl_new
Nov 24, 2025
Merged

updated hhl#1314
roie-d-classiq merged 3 commits intomainfrom
hhl_new

Conversation

@roie-d-classiq
Copy link
Collaborator

@roie-d-classiq roie-d-classiq commented Nov 20, 2025

PR Description

Some notes

  • Please make sure that the notebook runs successfully with the latest Classiq version.

  • Please make sure that you placed the files in an appropriate folder

    • And that the file names are clear, descriptive, and match the notebook content.
      • Note that we require the file names of .ipynb and .qmod to be unique across this repository.
    • Plus, please make sure that all required files are included: .qmod, .synthesis_options.json, .metadata.json
    • And that images are embedded inside the notebook, not added as external files
  • If applicable, please include link to the paper on which the notebook is based, in the notebook itself.

  • Please use rebase on your branch (no merge commits)

  • Please link this PR to the relevant issue

  • Please make sure to run pre-commit when commiting changes

    • If you're using git in the terminal, make sure to install pre-commit via running pip install pre-commit followed by pre-commit install
    • Note that Classiq runs automatic code linting. Meaning that one of the tests verifies the output of pre-commit.
    • Also note that pre-commit may minorly alter some files. Make sure to git add the changes done by pre-commit

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the link for condition number to the right place.

Correct latex for |vecb|.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the reference, didn't quite understand what was the problem with the latex?

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registers --> variables

\tilde{lambda}_j are the m bit approximations (correct n to m).

What is s?

C=1/2^m (correct n to m).

Maybe comment here that in principle, when \lambda-s do not have an exact m-binary representation, then there is also garbage for $|lambda \neq 0\rangle |1\rangle$.

You work here with the names memory and estimator, but in the code you use different namings. Maybe it is better to choose on wording, but I am not sure...


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modifed registers. Note that this is a very common term in the literature on quantum algorithms.

s is the sparsity of A, added a definition.

You're right, for a second it seemed weird to me that the minimum eigenvalue would depend on the phase accuracy, but now I see what is happening.

I'll modify the code accordingly.

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove this? The algorithm has 5 steps as written in the introduction.

Why the first step of preparing b deserves a subsection of its own, while all the other steps are given in a single block?


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure, I deleted the headline of the subsection

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

", returning a normalized solution" - I do not think this is actually the case here, since you do not do the final step of post-selecting ind=1.

which indicates the powers --> which indicates how the powers

number of phase qubits = m

 and indicates if the phase qubits return a normalized solution - I did not understand this sentence, how is it related to the phase qubits.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, fixed it.

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ambiguity is resolved by isolating the global phase. -- which ambiguity?


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ambiguity in the phase..., modified the sentence

Copy link
Collaborator Author

@roie-d-classiq roie-d-classiq Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the following section I modified the code..., this code was ori's version, I replaced it with my code performing the global phase identification using the scalar product....

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will define the inner quantum call to be used within the flexible QPE: a Suzuki Trotter of order one [3], whose repetitions parameter grows by some constant factor proportional to the evolution coefficient.

--> I think you repeat the info of this paragraph in the following paragraph.

estimator size m


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can replace the sparsity condition on A by any --> unfinished sentence.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can replace the sparsity condition on $A$ by any restriction allowing efficient Hamiltonian simulation.

Fixe the sentence

@@ -13,44 +13,84 @@
"id": "1",
Copy link
Member

@TomerGoldfriend TomerGoldfriend Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove # from the title text


Reply via ReviewNB

Copy link
Collaborator Author

@roie-d-classiq roie-d-classiq Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird, I don't see it..., it looks all right in my version

@roie-d-classiq roie-d-classiq merged commit 9e4a979 into main Nov 24, 2025
5 of 6 checks passed
@roie-d-classiq roie-d-classiq deleted the hhl_new branch November 24, 2025 11:15
@github-actions
Copy link

🔥 Great job, @roie-d-classiq! You've merged your 6th PR! 🎊

Your contributions to classiq-library are making a real difference. Keep up the fantastic work! 💪
Remember, every PR counts and helps improve the project. What will you tackle next? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants