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

Refactoring and debugging of Jordan Wigner transform (Issue #67) #82

Merged
merged 14 commits into from
Feb 12, 2025

Conversation

kvmto
Copy link
Collaborator

@kvmto kvmto commented Feb 10, 2025

This PR fixes #67.

Problem

The Jordan Wigner transform did not work correctly, in fact it generated hamiltonians with wrong coefficients or terms.

Changes

Changes so far have been applied to:

Rewritten from scratch jordan_wigner.cpp.
Added tests.
The original code from which the cpp version is created is https://github.com/quantumlib/OpenFermion/blob/v1.6.1/src/openfermion/transforms/opconversions/jordan_wigner.py#L128

Additional notes

The PR is not done yet! It requires more tests and a second look to the implementation.
The code has not been linted since much more changes are needed.

Findings

The implementation for molecule H4 now has correct energy values but higher number of terms compared to the python version.
Molecules like LiH do not achieve the correct energy values.

(Edited by @bmhowe23 given the latest updates from everyone)

Thanks to @Kenny-Heitritter and @james-qbraid for originally finding this issue!

@kvmto kvmto requested review from amccaskey and bmhowe23 February 10, 2025 13:47
bmhowe23 and others added 4 commits February 10, 2025 23:16
Co-authored-by: Alex McCaskey <[email protected]>
I, Ben Howe <[email protected]>, hereby add my Signed-off-by to this commit: 6ae55b2

Signed-off-by: Ben Howe <[email protected]>
Signed-off-by: Ben Howe <[email protected]>
…iltonian and added to the tests. This is much quicker test than converting to matrix and doing a dense diagonalization.

Signed-off-by: Scott Thornton <[email protected]>
Copy link

copy-pr-bot bot commented Feb 12, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@bmhowe23
Copy link
Collaborator

/ok to test

I, Ben Howe <[email protected]>, hereby add my Signed-off-by to this commit: c8fcc53

Signed-off-by: Ben Howe <[email protected]>
Signed-off-by: Ben Howe <[email protected]>
@bmhowe23
Copy link
Collaborator

/ok to test

@bmhowe23
Copy link
Collaborator

/ok to test

The Python test is exercising the C++ code, so we do not need a separate
C++ test for now.

Signed-off-by: Ben Howe <[email protected]>
Signed-off-by: Ben Howe <[email protected]>
@bmhowe23
Copy link
Collaborator

/ok to test

Copy link
Collaborator

@bmhowe23 bmhowe23 left a comment

Choose a reason for hiding this comment

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

Thanks, all!

@bmhowe23 bmhowe23 merged commit 323c592 into NVIDIA:main Feb 12, 2025
14 checks passed
@bmhowe23
Copy link
Collaborator

Thanks to @Kenny-Heitritter and @james-qbraid for originally finding the parent issue for this PR!

bmhowe23 added a commit to bmhowe23/cudaqx that referenced this pull request Feb 14, 2025
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.

Incorrect coefficients in Jordan-Wigner Transform
4 participants