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

[BUGFIX] Fix TF shape computation for pauli_decompose #4493

Merged
merged 9 commits into from
Aug 18, 2023

Conversation

obliviateandsurrender
Copy link
Contributor

@obliviateandsurrender obliviateandsurrender commented Aug 18, 2023

Context: There was an if condition inherited from the old legacy code where we were accessing shape via H.shape. This would fail for tensors built with TensorFlow.

Description of the Change: This PR fixes it by using qml.math.shape instead.

Benefits: Differentiability on TensorFlow backend.

Possible Drawbacks: N/A

Related GitHub Issues: N/A

@obliviateandsurrender obliviateandsurrender added this to the v0.32 milestone Aug 18, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #4493 (12044f7) into master (1762077) will decrease coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4493      +/-   ##
==========================================
- Coverage   99.71%   99.71%   -0.01%     
==========================================
  Files         378      378              
  Lines       34253    34242      -11     
==========================================
- Hits        34155    34144      -11     
  Misses         98       98              
Files Changed Coverage Δ
pennylane/operation.py 97.42% <ø> (-0.01%) ⬇️
pennylane/ops/functions/simplify.py 100.00% <ø> (ø)
pennylane/ops/op_math/prod.py 100.00% <ø> (ø)
pennylane/ops/qubit/state_preparation.py 100.00% <ø> (ø)
pennylane/transforms/batch_transform.py 100.00% <ø> (ø)
pennylane/transforms/split_non_commuting.py 100.00% <ø> (ø)
pennylane/_device.py 99.70% <100.00%> (-0.01%) ⬇️
pennylane/devices/qubit/adjoint_jacobian.py 100.00% <100.00%> (ø)
pennylane/devices/qubit/preprocess.py 100.00% <100.00%> (ø)
pennylane/gradients/general_shift_rules.py 100.00% <100.00%> (ø)
... and 16 more

@timmysilv
Copy link
Contributor

could you add a quick test for what was failing before? maybe.. do it for jax/torch as well just to be extra sure? otherwise this sounds good to me 👍

Copy link
Contributor

@timmysilv timmysilv left a comment

Choose a reason for hiding this comment

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

nice find!

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 💯

@obliviateandsurrender obliviateandsurrender added the merge-ready ✔️ All tests pass and the PR is ready to be merged. label Aug 18, 2023
@obliviateandsurrender obliviateandsurrender merged commit c2995b6 into master Aug 18, 2023
39 checks passed
@obliviateandsurrender obliviateandsurrender deleted the fix-pauli-dec branch August 18, 2023 18:06
mlxd pushed a commit that referenced this pull request Aug 23, 2023
* fix `shape` bug

* update `PR` number

* trigger

* change `type`

* add more tests

* parametrize prettify

* suggestions
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.

3 participants