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

Deprecate old QNode #440

Merged
merged 33 commits into from
Dec 6, 2019
Merged

Deprecate old QNode #440

merged 33 commits into from
Dec 6, 2019

Conversation

antalszava
Copy link
Contributor

Context:
Currently, the refactored QNode is in the pennylane/beta folder and users could potentially use it through referencing there. However, maintaining two types of QNode objects is not favorable in the long run. Therefore, deprecation of the old-style QNode is needed.

Description of the Change:
Deprecates the old QNode such that only the new version is supported. Moves all related files from the beta folder.

Benefits:
Only the new QNode version needs to be maintained.

Possible Drawbacks:
New syntax to be used e.g. for the decorator, several changes need to be noted.

Related GitHub Issues:
Series of previous QNode PRs, such as #420, #425

@antalszava antalszava added the WIP 🚧 Work-in-progress label Dec 5, 2019
@codecov
Copy link

codecov bot commented Dec 5, 2019

Codecov Report

Merging #440 into master will decrease coverage by 0.09%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #440     +/-   ##
=========================================
- Coverage   99.33%   99.24%   -0.1%     
=========================================
  Files          46       45      -1     
  Lines        3760     3303    -457     
=========================================
- Hits         3735     3278    -457     
  Misses         25       25
Impacted Files Coverage Δ
pennylane/qnodes/device_jacobian.py 90.9% <ø> (ø)
pennylane/beta/__init__.py 100% <ø> (ø) ⬆️
pennylane/interfaces/autograd.py 100% <ø> (ø)
pennylane/qnodes/__init__.py 100% <ø> (ø)
pennylane/qnodes/cv.py 100% <ø> (ø)
pennylane/beta/plugins/expt_tensornet_tf.py 92.1% <ø> (ø) ⬆️
pennylane/qnodes/decorator.py 100% <ø> (ø)
pennylane/qnodes/qubit.py 100% <ø> (ø)
pennylane/interfaces/__init__.py 100% <ø> (ø)
pennylane/qnodes/jacobian.py 100% <100%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 375d25a...f57ec37. Read the comment docs.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

This is great Antal, thanks for helping finish this! I can't seem to compile the documentation locally, so I'm going to see if I can fix that quickly. Once done, this should be ready for merging.

Before I forget, let's remember to update the changelog as well 😆

pennylane/beta/vqe/vqe.py Show resolved Hide resolved
pennylane/qnodes/base.py Show resolved Hide resolved
tests/beta/test_vqe.py Show resolved Hide resolved
tests/interfaces/test_tf.py Outdated Show resolved Hide resolved
tests/test_quantum_gradients.py Show resolved Hide resolved
tests/test_templates_layers.py Outdated Show resolved Hide resolved
tests/test_templates_subroutines.py Show resolved Hide resolved
tests/test_templates_subroutines.py Show resolved Hide resolved
@josh146
Copy link
Member

josh146 commented Dec 6, 2019

Update: I have the docs correctly compiling, and viewable here

@antalszava
Copy link
Contributor Author

antalszava commented Dec 6, 2019

That is really nice @josh146 , thanks!

A really nasty caveat that I had to double-check (my latest commits include a correction to these cases) is plainly exchanging a circuit with a OperationRecorder the following way:

def circuit(weights):
    CVNeuralNetLayers(*weights, wires=range(4))
    return qml.expval(qml.X(wires=0))
with qml.utils.OperationRecorder() as rec:
    CVNeuralNetLayers(*weights, wires=range(4))
    return qml.expval(qml.X(wires=0))

Extra care needs to be taken, as in the second case the test will simply pass (due to it reaching the return statement) without really checking the test case itself (as most of the times this code snippet would be placed at the beginning of the test).

This is the reason why the return statements were (and should be) removed when using the OperationRecorder.

Just thought it's worth pointing this out just as a future use of the OperationRecorder.

@josh146 josh146 merged commit 493d419 into master Dec 6, 2019
@josh146 josh146 deleted the deprecate_old_qnode branch December 6, 2019 21:21
@josh146 josh146 mentioned this pull request Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP 🚧 Work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants