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

Make TQ Compatible with Qiskit ≥1.0.0 #267

Merged

Conversation

king-p3nguin
Copy link

@king-p3nguin king-p3nguin commented May 29, 2024

Description

Resolve #262

Notes

  • qiskit.tools.job_monitor has been removed from qiskit and there is no replacement (ref: Migration guides), so I just removed from the code.

(I am contributing to this project as a unitaryHACK participant)

@01110011011101010110010001101111
Copy link
Collaborator

Hey, if you could take a look at the tests to get them functional, that would be amazing! Don’t worry about getting the lint and pre-commit working (they generally won’t pass), just get the Python package tests functional (which should essentially be the same thing from running pytest)

Right now, it looks like there’s a small syntax error:

AttributeError: 'Qubit' object has no attribute 'index'. Did you mean: '_index'?

You may also need to change the extra dependencies in the workflow here. (We may actually be able to delete that line.)

@king-p3nguin
Copy link
Author

Thanks for being patient. I ran the tests and examples locally and they should be working now.
It seems like sometimes the test using random numbers fails, so I increased the threshold of assertion.

@01110011011101010110010001101111 01110011011101010110010001101111 changed the base branch from main to dev May 29, 2024 18:30
@01110011011101010110010001101111
Copy link
Collaborator

Awesome, the PR functionally looks great! I’ll do a more thorough review later today then we can merge. Thanks so much!

@01110011011101010110010001101111
Copy link
Collaborator

01110011011101010110010001101111 commented May 30, 2024

I’ll get to this ASAP, sorry about the delay! In the meantime though, could you comment on the issue so I can more easily assign you it? That is necessary for making sure you get the bounty at the end of Unitary Hack!

@king-p3nguin
Copy link
Author

Hi @GenericP3rson, it seems like there are still some places I have to change to merge the dev branch. Would you like to review the code first, or should I resolve the remaining compatibility issues first?

@01110011011101010110010001101111
Copy link
Collaborator

Don't worry about it for now, I'm going to cherrypick the code that is failing out (since it's mostly density matrix code that isn't ready for the dev branch yet) and merge into the updated branch.

@01110011011101010110010001101111 01110011011101010110010001101111 changed the base branch from dev to main June 2, 2024 20:20
@01110011011101010110010001101111 01110011011101010110010001101111 changed the base branch from main to unitary-hack June 2, 2024 20:21
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, this looks good to me! One small note to relax the dependencies but otherwise will be good to merge!

requirements.txt Outdated Show resolved Hide resolved
@01110011011101010110010001101111
Copy link
Collaborator

Looks like I messed something up while rebasing — I’ll fix that up!

requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just one more instance of the unnecessary cloning.

torchquantum/operator/standard_gates/qubit_unitary.py Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, looks good from my end!

@Hanrui-Wang Hanrui-Wang merged commit ad384fa into mit-han-lab:unitary-hack Jun 12, 2024
5 of 7 checks passed
@king-p3nguin king-p3nguin deleted the bump-up-qiskit-version branch June 12, 2024 14:37
@gurinder-hub
Copy link

Is TQ now compatible with qiskit>=1.1.0?

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.

Make TQ Compatible with Qiskit ≥1.0.0
4 participants