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

Add Minimum Vertex Cover in example #574

Closed
wants to merge 1 commit into from

Conversation

ianmalcolm
Copy link

Hi @scarrazza,

May I have your comments on this pull request, please?

Many thanks!

@scarrazza
Copy link
Member

@ianmalcolm thanks for this PR. We had discussed about this implementation today, and we would like to check with you if the qubo_mvc function could return a qibo Hamiltonian object. We believe this could improve the usability of the MVC code.

@ianmalcolm
Copy link
Author

@ianmalcolm thanks for this PR. We had discussed about this implementation today, and we would like to check with you if the qubo_mvc function could return a qibo Hamiltonian object. We believe this could improve the usability of the MVC code.

Yes, it is possible. I will change the function to have a qibo Hamiltonian object.

@ianmalcolm
Copy link
Author

ianmalcolm commented May 17, 2022

7a88381 and 24a5b18 seems to be an answer to #430, but subject to further test and verification.

@@ -0,0 +1,201 @@
```python
Copy link
Member

Choose a reason for hiding this comment

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

@ianmalcolm thanks for this. I think we can keep this README and remove the jupyter notebook.

Copy link
Author

Choose a reason for hiding this comment

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

demo.ipynb is removed in 604d971

@@ -0,0 +1,113 @@
from typing import Dict, List, Tuple, Union, Any, Optional, Generator, Iterator, Hashable

def binary2spin(linear: Dict[int, float],
Copy link
Member

Choose a reason for hiding this comment

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

@ianmalcolm following our discussion today, let me suggest to remove binary2spin and keep just a single function graph2hamiltonian(graph, penalty).

Copy link
Author

Choose a reason for hiding this comment

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

In f470579 I added a function hamiltonian_mvc, which creates a qibo hamiltonian from a networkx graph. The test of this function is added through f8f398f. In hamiltonian_mvc I first create qubo version of mvc then convert it to hamiltonian. So it still depends on binary2spin and spin2hamiltonian.

Copy link
Member

@scarrazza scarrazza left a comment

Choose a reason for hiding this comment

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

@ianmalcolm, we have to rebase this branch from master.
Could you please open another PR from master directly from this repository (no fork)?

Add utils to convert between binary and spin models

Ensure all decision variables appear in the linear terms. In rare cases, a variable could be ignored if it has zero bias and iteractions and would be missing in final solutions and causes problems.

Add a negative sign to hamiltonian

Add the usage of converting mvc problem to hamiltonian and an example of adiabatic time evolution.

Add tests

Add test entry for mvc

Add hamiltonian_mvc, which directly creates a hamiltonian from a networkx graph

Remove duplicated documentaiton

Add test on hamiltonian_mvc

Revert "Remove duplicated documentaiton"

This reverts commit 1decc38.

Remove duplicated documentation
@ianmalcolm
Copy link
Author

ianmalcolm commented Jun 8, 2022

@ianmalcolm, we have to rebase this branch from master. Could you please open another PR from master directly from this repository (no fork)?

Hi @scarrazza I just rebase my mvc branch in my fork repo to the latest master. Do you mean I should create a mvc branch in the upstream repo and open a new PR?

@scarrazza
Copy link
Member

@ianmalcolm thanks, yes please open another PR but using the qibo repository as baseline instead of your fork (i.e. clone the qibo repository and move your branch changes to a new PR.

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