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 from_openfermion function for fermionic operators #5808

Merged
merged 35 commits into from
Jun 14, 2024

Conversation

ddhawan11
Copy link
Contributor

Context:
Added function to convert openfermion fermionic operator to pennylane fermionic operators

Description of the Change:
This function converts openfermion FermionOperator object to pennylane FermiWord and FermiSentence objects

Benefits:
Easier integration between pennylane and openfermion

Possible Drawbacks:

Related GitHub Issues:

@ddhawan11 ddhawan11 changed the title Add from_openfermion function for fermionic operators [WIP] Add from_openfermion function for fermionic operators Jun 6, 2024
@ddhawan11
Copy link
Contributor Author

[sc-62839]

Copy link

codecov bot commented Jun 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (cdc36bc) to head (762e112).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5808      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         420      421       +1     
  Lines       40170    39893     -277     
==========================================
- Hits        40042    39763     -279     
- Misses        128      130       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ddhawan11 ddhawan11 requested a review from soranjh June 7, 2024 12:52
@ddhawan11 ddhawan11 marked this pull request as ready for review June 7, 2024 12:52
@ddhawan11 ddhawan11 changed the title [WIP] Add from_openfermion function for fermionic operators Add from_openfermion function for fermionic operators Jun 7, 2024
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @ddhawan11, this looks great!

One question, could we make the functionality available at the top level? qml.from_openfermion()?

pennylane/qchem/convert_fermionic.py Outdated Show resolved Hide resolved
@trbromley
Copy link
Contributor

Are we waiting on this PR before we finalize #5773?

@soranjh
Copy link
Contributor

soranjh commented Jun 10, 2024

Are we waiting on this PR before we finalize #5773?

No, these two PRs are independent, but complement each other.

Copy link
Contributor

@soranjh soranjh left a comment

Choose a reason for hiding this comment

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

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

pennylane/qchem/convert_fermionic.py Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/qchem/convert_fermionic.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_fermionic.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_fermionic.py Outdated Show resolved Hide resolved
ddhawan11 and others added 3 commits June 11, 2024 07:46
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
@trbromley
Copy link
Contributor

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

@soranjh I asked this because I see both PRs working on from_openfermion functions. But is the subtlety that #5773 works on qml.from_openfermion and this PR works on qml.qchem.from_openfermion? I'm wondering if this makes sense, and whether it could work to have the same qml.from_openfermion function work for both usecases.

@ddhawan11
Copy link
Contributor Author

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

@soranjh I asked this because I see both PRs working on from_openfermion functions. But is the subtlety that #5773 works on qml.from_openfermion and this PR works on qml.qchem.from_openfermion? I'm wondering if this makes sense, and whether it could work to have the same qml.from_openfermion function work for both usecases.

@trbromley The difference between two functions is that this one converts Openfermion fermionic operator to PennyLane operators whereas the other PR works on converting openfermion qubit operators to PennyLane operators. I hope this helps.

@ddhawan11 ddhawan11 requested a review from soranjh June 12, 2024 13:43
@soranjh
Copy link
Contributor

soranjh commented Jun 12, 2024

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

@soranjh I asked this because I see both PRs working on from_openfermion functions. But is the subtlety that #5773 works on qml.from_openfermion and this PR works on qml.qchem.from_openfermion? I'm wondering if this makes sense, and whether it could work to have the same qml.from_openfermion function work for both usecases.

@trbromley We should have two functions qml.from_openfermion and qml.to_openfermion. I will make sure both PRs work this way. Just a note that the functionality in these PRs are NOT duplicated; what we have here is not added there.

ddhawan11 and others added 7 commits June 13, 2024 04:13
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Copy link
Contributor

@Jaybsoni Jaybsoni left a comment

Choose a reason for hiding this comment

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

Nice work 💯! Looks good to me, just a few comments to address

Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
@trbromley
Copy link
Contributor

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

@soranjh I asked this because I see both PRs working on from_openfermion functions. But is the subtlety that #5773 works on qml.from_openfermion and this PR works on qml.qchem.from_openfermion? I'm wondering if this makes sense, and whether it could work to have the same qml.from_openfermion function work for both usecases.

@trbromley We should have two functions qml.from_openfermion and qml.to_openfermion. I will make sure both PRs work this way. Just a note that the functionality in these PRs are NOT duplicated; what we have here is not added there.

Thanks! Ok, I'll hold off on reviewing this PR until it's ready.

@soranjh
Copy link
Contributor

soranjh commented Jun 13, 2024

Thanks @ddhawan11, looks good overall. Can we directly construct the objects as I suggested in the code?

@soranjh I asked this because I see both PRs working on from_openfermion functions. But is the subtlety that #5773 works on qml.from_openfermion and this PR works on qml.qchem.from_openfermion? I'm wondering if this makes sense, and whether it could work to have the same qml.from_openfermion function work for both usecases.

@trbromley We should have two functions qml.from_openfermion and qml.to_openfermion. I will make sure both PRs work this way. Just a note that the functionality in these PRs are NOT duplicated; what we have here is not added there.

Thanks! Ok, I'll hold off on reviewing this PR until it's ready.

@trbromley This PR (5808) is ready for your review right now.

Copy link
Contributor

@soranjh soranjh left a comment

Choose a reason for hiding this comment

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

Thanks @ddhawan11, one minor comment on using a full name for openfermion_op and then good from my side.

pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
ddhawan11 and others added 5 commits June 13, 2024 12:18
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
pennylane/qchem/convert_openfermion.py Outdated Show resolved Hide resolved
ddhawan11 and others added 3 commits June 14, 2024 09:02
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
@ddhawan11 ddhawan11 merged commit 9b3060f into master Jun 14, 2024
40 checks passed
@ddhawan11 ddhawan11 deleted the pennylane_openfermion_integration branch June 14, 2024 13:30
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

4 participants