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

Update XY_mixer_hamiltonian with Additional mixer_qubit_conndectivity #313

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

yoshioka1128
Copy link

@yoshioka1128 yoshioka1128 commented May 31, 2024

Description

This PR adds new mixer_qubit_connectivity to the XY_mixer_hamiltonian and get_mixer_hamiltonian function.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code and used numpy-style docstrings
  • I have made corresponding updates to the documentation.
  • My changes generate no new warnings
  • I have added/updated tests to make sure bugfix/feature works.
  • New and existing unit tests pass locally with my changes.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Add the "cyclic" case to the code in test_utilities.py.

@KilianPoirier KilianPoirier self-requested a review May 31, 2024 08:57
Copy link
Collaborator

@KilianPoirier KilianPoirier left a comment

Choose a reason for hiding this comment

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

Changes look great! Thanks for implementing the cyclic version of the XY mixer hamiltonian.

The only comment I have is to update the test here.
Basically adding adding an extra "cyclic" case to the following code:

for connectivity in connectivities:
  # Define connectivity explicit indexing
  if connectivity == "full":
    terms_indices = list(itertools.combinations(range(n_qubits), 2))
    input_coefficients = 2 * [1 for _ in range(len(terms_indices))]
    mixer_type = "xy"
  
  elif connectivity == "chain":
    terms_indices = [(i, i + 1) for i in range(n_qubits - 1)]
    input_coefficients = 2 * [1 for _ in range(len(terms_indices))]
    mixer_type = "xy"
  
  elif connectivity == "star":
    terms_indices = [(0, i) for i in range(1, n_qubits)]
    input_coefficients = 2 * [1 for _ in range(len(terms_indices))]
    mixer_type = "xy"

  else:
    input_coefficients = [1 for _ in range(n_qubits)]
    mixer_type = "x"

Let me know what you think of this!

@yoshioka1128
Copy link
Author

Thank you for your feedback! I think adding the cyclic case to the test is a great idea. I will proceed with the following updates to the test code:

  1. Add the "cyclic" case to the connectivities list.
  2. Implement the necessary logic for the cyclic connectivity in the loop.

I'll make these changes and update the PR shortly. Please let me know if you have any additional suggestions or concerns.

@yoshioka1128
Copy link
Author

yoshioka1128 commented Jun 2, 2024

I have added the cyclic connectivity test cases as requested.

Here are the test results:

........................
Qubits in the specified Hamiltonian are remapped to [0]. Please specify the physical quantum register as a qubit layout argument in the backend.
Qubits in the specified Hamiltonian are remapped to [0, 1, 2]. Please specify the physical quantum register as a qubit layout argument in the backend.
...
----------------------------------------------------------------------
Ran 27 tests in 0.294s

OK

All tests passed successfully.
The warnings about qubit remapping are not errors and do not affect the test results.
Please let me know if there are any further changes or additional tests needed.

Copy link
Collaborator

@KilianPoirier KilianPoirier left a comment

Choose a reason for hiding this comment

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

The code looks great!
Thank you for implementing the cyclic version of the XY-mixer Hamiltonian

@KilianPoirier KilianPoirier merged commit 002f527 into entropicalabs:dev Aug 29, 2024
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.

None yet

2 participants