Update deprecated QNode.(q)tape property usage in tests
#6625
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
andrijapau
changed the title
QNode.qtape/tape property deprecationQNode.qtape/tape property
andrijapau
changed the title
QNode.qtape/tape propertyQNode.qtape/tape property
andrijapau
changed the title
QNode.qtape/tape propertyQNode.qtape/tape property
andrijapau
changed the title
QNode.qtape/tape propertyQNode.qtape/tape property
andrijapau
changed the title
QNode.qtape/tape propertyQNode.qtape/tape property usage
andrijapau
changed the title
QNode.qtape/tape property usageQNode.qtape/tape property usage in tests
andrijapau
changed the title
QNode.qtape/tape property usage in testsQNode.(q)tape property usage in tests
There was a problem hiding this comment.
This looks very good to me!
About the deletion(s) in the tests/workflow/interfaces/qnode/ file, I might miss context here, but it seems to me that you are deleting all the code lines checking properties of the QuantumScript (such as trainable_params) that it was possible to access from the QNode using the deprecated tape/qtape property (for example as circuit.qtape.trainable_params).
Now that the tape/qtape property is deprecated, we no longer have access to such properties of the QuantumScript from the QNode interface, hence the decision to remove the associated test lines. Is that correct?
tests/gradients/parameter_shift/test_parameter_shift_hessian.py
Outdated
Show resolved
Hide resolved
Indeed 🙂. I personally think that the deletion of these lines doesn't impact the overall test quality. More extensive testing of the ML interface gradient logic is being done in |
…es (as per Pietro's suggestion)
There was a problem hiding this comment.
@andrijapau Thanks for the clarification. I agree, this seems a totally legitimate choice to me.
I left just some personal preference in terms of names used in the tests 🚀
Co-authored-by: Pietropaolo Frisoni <[email protected]>
Co-authored-by: Pietropaolo Frisoni <[email protected]>
Co-authored-by: Pietropaolo Frisoni <[email protected]>
**Context:** Follow up to #6625. For some reason, CI never complained about this property usage despite no `qml.PennylaneDeprecationWarning` suppression occuring in these files ... weird. Created story ([sc-80957]) to track. **Description of the Change:** Tests should construct the tape dynamically using `construct_tape` rather than use the deprecated properties. **Benefits:** Better testing and code maintainability. **Possible Drawbacks:** None [[sc-78317](https://app.shortcut.com/xanaduai/story/78317)] --------- Co-authored-by: Pietropaolo Frisoni <[email protected]>
Context:
Many tests incorrectly rely on the
tapeandqtapeproperties to access information during tests. With these properties deprecated in #6583, deprecation warnings in the affected test files have been suppressed. This PR aims to replace all instances of deprecated code with the correct code.Description of the Change:
Tests should construct the tape dynamically using
construct_taperather than use the deprecated properties.Benefits: Better testing and code maintainability.
Possible Drawbacks:
I’m still a bit unsure about the changes in tests/workflow/interfaces/qnode/. The lines I deleted were originally a workaround to further verify QNode interfacing. However, the primary goal of the tests in this folder isn’t to check gradient accuracy but rather to ensure proper interfacing with the QNode architecture (e.g., verifying data types and result shapes). Since the property being used should now be considered private, relying on it in tests isn’t best practice—hence the deletion.
[sc-78317]