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

Adding adjoint methods to FermiWord and FermiSentence classes #6166

Merged
merged 29 commits into from
Sep 10, 2024

Conversation

willjmax
Copy link
Contributor

@willjmax willjmax commented Aug 28, 2024

This PR adds a .adjoint() method to the FermiWord and FermiSentence classes. In both cases the method can be used in the following way: f_dag = f.adjoint(). The resulting f_dag is a FermiWord or FermiSentence corresponding to the adjoint of f. This PR addresses [sc-72055].

Copy link

codecov bot commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (b0a9c61) to head (07a9cb3).
Report is 338 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6166   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         445      445           
  Lines       42417    42441   +24     
=======================================
+ Hits        42261    42285   +24     
  Misses        156      156           

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

Copy link
Contributor

@obliviateandsurrender obliviateandsurrender left a comment

Choose a reason for hiding this comment

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

Thanks @willjmax, leaving some suggestions.

pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
pennylane/fermi/fermionic.py Show resolved Hide resolved
pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
Copy link
Contributor

@austingmhuang austingmhuang left a comment

Choose a reason for hiding this comment

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

Looking good @willjmax.

Do we need to add test cases for FermiWord specifically? I see the tests for FermiSentence but it might be useful to have unit tests for adjoint() of FermiWord as well.

tests/fermi/test_fermionic.py Outdated Show resolved Hide resolved
@willjmax
Copy link
Contributor Author

willjmax commented Sep 4, 2024

Looking good @willjmax.

Do we need to add test cases for FermiWord specifically? I see the tests for FermiSentence but it might be useful to have unit tests for adjoint() of FermiWord as well.

There are tests for both FermiWord and FermiSentence.

@austingmhuang
Copy link
Contributor

Looking good @willjmax.
Do we need to add test cases for FermiWord specifically? I see the tests for FermiSentence but it might be useful to have unit tests for adjoint() of FermiWord as well.

There are tests for both FermiWord and FermiSentence.

My bad, think I scrolled too quickly 😅. Looks good to me for the most part. Could add docstrings for the new tests but they seem straightforward enough to me that I personally wouldn't require them. @obliviateandsurrender are docstrings a hard requirement or is it ok to omit them for really simple tests?

Copy link
Contributor

@austingmhuang austingmhuang 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 @willjmax

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
willjmax and others added 6 commits September 5, 2024 16:34
Co-authored-by: Austin Huang <65315367+austingmhuang@users.noreply.github.com>
Co-authored-by: Austin Huang <65315367+austingmhuang@users.noreply.github.com>
Copy link
Contributor

@obliviateandsurrender obliviateandsurrender left a comment

Choose a reason for hiding this comment

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

Looks good, just some minor suggestions.

pennylane/fermi/fermionic.py Show resolved Hide resolved
pennylane/fermi/fermionic.py Show resolved Hide resolved
tests/fermi/test_fermionic.py Show resolved Hide resolved
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
@willjmax willjmax added the merge-ready ✔️ All tests pass and the PR is ready to be merged. label Sep 6, 2024
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 @willjmax, left some documentation comments.

pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
pennylane/fermi/fermionic.py Outdated Show resolved Hide resolved
pennylane/fermi/fermionic.py Show resolved Hide resolved
willjmax and others added 9 commits September 9, 2024 15:12
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>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
@willjmax willjmax enabled auto-merge (squash) September 10, 2024 12:08
@willjmax willjmax merged commit c14efd4 into PennyLaneAI:master Sep 10, 2024
37 checks passed
mudit2812 pushed a commit that referenced this pull request Sep 10, 2024
This PR adds a `.adjoint()` method to the `FermiWord` and
`FermiSentence` classes. In both cases the method can be used in the
following way: `f_dag = f.adjoint()`. The resulting `f_dag` is a
`FermiWord` or `FermiSentence` corresponding to the adjoint of `f`. This
PR addresses [sc-72055].

---------

Co-authored-by: Austin Huang <65315367+austingmhuang@users.noreply.github.com>
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
mudit2812 pushed a commit that referenced this pull request Sep 12, 2024
This PR adds a `.adjoint()` method to the `FermiWord` and
`FermiSentence` classes. In both cases the method can be used in the
following way: `f_dag = f.adjoint()`. The resulting `f_dag` is a
`FermiWord` or `FermiSentence` corresponding to the adjoint of `f`. This
PR addresses [sc-72055].

---------

Co-authored-by: Austin Huang <65315367+austingmhuang@users.noreply.github.com>
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
mudit2812 pushed a commit that referenced this pull request Sep 16, 2024
This PR adds a `.adjoint()` method to the `FermiWord` and
`FermiSentence` classes. In both cases the method can be used in the
following way: `f_dag = f.adjoint()`. The resulting `f_dag` is a
`FermiWord` or `FermiSentence` corresponding to the adjoint of `f`. This
PR addresses [sc-72055].

---------

Co-authored-by: Austin Huang <65315367+austingmhuang@users.noreply.github.com>
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-ready ✔️ All tests pass and the PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants