Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove parser #92

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

remove parser #92

wants to merge 25 commits into from

Conversation

Edoardo-Pedicillo
Copy link
Collaborator

@Edoardo-Pedicillo Edoardo-Pedicillo commented Oct 16, 2024

@Edoardo-Pedicillo Edoardo-Pedicillo marked this pull request as ready for review October 16, 2024 06:49
Copy link
Collaborator

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

Thanks @Edoardo-Pedicillo, just a few minor changes.

README.md Outdated Show resolved Hide resolved
src/boostvqe/boost.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/boostvqe/boost.py Outdated Show resolved Hide resolved
src/boostvqe/boost.py Show resolved Hide resolved
Learning rate decay factor used when the optimizer is SGD (stochastic gradient descent).

nboost (int, default: 1):
Number of times DBI (Deterministic Boost Iteration) is applied in the optimization process.
Copy link
Contributor

Choose a reason for hiding this comment

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

DBI = double-bracket iteration

is this part of the CMA optimizer?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the number of times DBQA is used along a single, long, VQE training.


if args.optimizer_options is None:
opt_options = {}
dbi_steps (int, default: 1):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dbi_steps (int, default: 1):
dbqa_steps (int, default: 1):

this code should be capable of running DBI (dense matrices) and GCI (qibo.Circuit)

dbi_steps (int, default: 1):
Number of DBI iterations performed each time DBI is called.

stepsize (float, default: 0.01):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
stepsize (float, default: 0.01):
dbr_duration (float, default: 0.01):

Each DBI step is DBR (double-bracket rotation) and in the strategies proceeding we always say DBR duration

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the stepsize has been removed from the arguments, but still remains in the docstrings. One of the two things should be solved.

if args.optimizer_options is None:
opt_options = {}
dbi_steps (int, default: 1):
Number of DBI iterations performed each time DBI is called.
Copy link
Contributor

Choose a reason for hiding this comment

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

iterations of what?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Number of rotations, namely DBQA steps.

store_h (bool, default: False):
If this flag is set, the Hamiltonian `H` is stored at each iteration.

hamiltonian (str, default: "XXZ"):
Copy link
Contributor

Choose a reason for hiding this comment

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

after merging #77 this can become also a symbolic Hamiltonian

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is also proposed by @Edoardo-Pedicillo at line 148.


mode:
Define the DBI Generator.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""
"""please_be_verbose:
Flag which switches on verbose reporting - after each DBQA step a report is printed on energy gain, fidelity and circuit depth and stored in the output_folder as a log file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also this should be done in a separate PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's move ahead without it

README.md Show resolved Hide resolved
@marekgluza marekgluza removed the request for review from Sam-XiaoyueLi November 5, 2024 05:18
@marekgluza
Copy link
Contributor

Thank you @Edoardo-Pedicillo !

I like the overall code structure and e.g. the clarity of the new Readme.

I removed Sam as reviewer:

  • agree with @MatteoRobbiati and @andrea-pasquale if the coding approach works for you
  • then merge to finish this refactoring
  • I'll move ahead with the journal submission if you confirm that the code can reproduce what we had in plots
  • the README example is very small so we should have a simple documentation script/notebook which additionally explains how to extract the energy gain and circuit depth

@marekgluza
Copy link
Contributor

Will this close #83 where the number of trotter steps and dbqa steps should be different?

src/boostvqe/boost.py Outdated Show resolved Hide resolved
Learning rate decay factor used when the optimizer is SGD (stochastic gradient descent).

nboost (int, default: 1):
Number of times DBI (Deterministic Boost Iteration) is applied in the optimization process.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the number of times DBQA is used along a single, long, VQE training.

if args.optimizer_options is None:
opt_options = {}
dbi_steps (int, default: 1):
Number of DBI iterations performed each time DBI is called.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Number of rotations, namely DBQA steps.

store_h (bool, default: False):
If this flag is set, the Hamiltonian `H` is stored at each iteration.

hamiltonian (str, default: "XXZ"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this is also proposed by @Edoardo-Pedicillo at line 148.

dbi_steps (int, default: 1):
Number of DBI iterations performed each time DBI is called.

stepsize (float, default: 0.01):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the stepsize has been removed from the arguments, but still remains in the docstrings. One of the two things should be solved.

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.

main.py runs only in single commutator mode - there is no GCI on main branch right now
4 participants