[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

Warnings for two_qubit_unitary #6437

Merged
merged 10 commits into from
Oct 25, 2024
Merged

Conversation

JerryChen97
Copy link
Contributor
@JerryChen97 JerryChen97 commented Oct 23, 2024

Context:
qml.ops.op_math.decompositions.two_qubit_unitary.py is not differentiable.
Description of the Change:
Will raise warning if users try call this method in differentiable way
Benefits:
Raise caution regarding the non-differentiability of affected program
Temporarily help prevent usage of this method in differentiable way until full implementation in the future
Possible Drawbacks:
Not fully fix the affected issue.
Related GitHub Issues:
#6045

Related Shortcut Stories:
[sc-69959]

Note: This is only for suppressing static check of differentiability.
The essential difficulty of this method to be differentiable persists.
Further warnings will be included in the same script.
@JerryChen97 JerryChen97 added bug 🐛 Something isn't working WIP 🚧 Work-in-progress labels Oct 23, 2024
@JerryChen97 JerryChen97 self-assigned this Oct 23, 2024
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@JerryChen97
Copy link
Contributor Author

The replacement of the eigvals call with shorter, conciser condition tester during the branching within _compute_num_cnots won't help too much over the original issue. The only effect is there will be no more NotImplementedError, but the overall non-differentiability issue persists.

@JerryChen97 JerryChen97 marked this pull request as ready for review October 23, 2024 20:18
@JerryChen97 JerryChen97 requested a review from albi3ro October 23, 2024 20:20
@JerryChen97 JerryChen97 changed the title [WIP] replace eigvals with equivalent eig test replace eigvals with equivalent eig test Oct 23, 2024
@JerryChen97 JerryChen97 removed the WIP 🚧 Work-in-progress label Oct 23, 2024
Copy link
codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.39%. Comparing base (43cb979) to head (83f9466).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6437   +/-   ##
=======================================
  Coverage   99.38%   99.39%           
=======================================
  Files         450      450           
  Lines       42619    42624    +5     
=======================================
+ Hits        42359    42364    +5     
  Misses        260      260           

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

@JerryChen97 JerryChen97 changed the title replace eigvals with equivalent eig test Warnings for two_qubit_unitary Oct 23, 2024
Copy link
Contributor
@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

:+1

@PietropaoloFrisoni PietropaoloFrisoni self-requested a review October 24, 2024 19:34
Copy link
Contributor
@PietropaoloFrisoni PietropaoloFrisoni left a comment

Choose a reason for hiding this comment

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

Looks great to me! Just a totally optional suggestion

pennylane/ops/op_math/decompositions/two_qubit_unitary.py Outdated Show resolved Hide resolved
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
@JerryChen97
Copy link
Contributor Author

Thanks a lot for reviews! I'll mod the changelog and merge

@JerryChen97 JerryChen97 enabled auto-merge (squash) October 25, 2024 16:07
@JerryChen97 JerryChen97 merged commit 580daba into master Oct 25, 2024
40 checks passed
@JerryChen97 JerryChen97 deleted the fix-vjp-eigvals-issue-#6045 branch October 25, 2024 17:46
mudit2812 pushed a commit that referenced this pull request Nov 11, 2024
Note: This is only for suppressing static check of differentiability.
The essential difficulty of this method to be differentiable persists.
Further warnings will be included in the same script.

### Before submitting

Please complete the following checklist when submitting a PR:

- [x] All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to
the
      test directory!

- [x] All new functions and code must be clearly commented and
documented.
If you do make documentation changes, make sure that the docs build and
      render correctly by running `make docs`.

- [x] Ensure that the test suite passes, by running `make test`.

- [ ] Add a new entry to the `doc/releases/changelog-dev.md` file,
summarizing the
      change, and including a link back to the PR.

- [x] The PennyLane source code conforms to
      [PEP8 standards](https://www.python.org/dev/peps/pep-0008/).
We check all of our code against [Pylint](https://www.pylint.org/).
      To lint modified files, simply `pip install pylint`, and then
      run `pylint pennylane/path/to/file.py`.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


------------------------------------------------------------------------------------------------------------

**Context:**
`qml.ops.op_math.decompositions.two_qubit_unitary.py` is not
differentiable.
**Description of the Change:**
Will raise warning if users try call this method in differentiable way
**Benefits:**
Raise caution regarding the non-differentiability of affected program
Temporarily help prevent usage of this method in differentiable way
until full implementation in the future
**Possible Drawbacks:**
Not fully fix the affected issue.
**Related GitHub Issues:**
#6045 

**Related Shortcut Stories:**
[sc-69959]

---------

Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants