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

Add TransformTrace, TransformTracer and TransformInterpreter classes for transforming PLxPR #6389

Draft
wants to merge 179 commits into
base: master
Choose a base branch
from

Conversation

mudit2812
Copy link
Contributor

@mudit2812 mudit2812 commented Oct 11, 2024

This PR adds 3 new classes to qml.capture to facilitate transforming PLxPR natively without the need to first create QuantumScripts.

  • TransformInterpreter will be used to evaluate PLxPR for the purposes of applying transforms.
  • TransformInterpreter will use TransformTrace and TransformTracer to transform primitives that are being evaluated.
  • Scaffolding needed to create transforms for PLxPR has been added to TransformDispatcher, TransformContainer, and TransformProgram.
  • A markdown file has been added to qml.capture to give a detailed explanation of how the framework for applying transforms natively to PLxPR works.
  • Update qml.capture.enable() and qml.capture.disable() to dispatch to jax when using pennylane classes with capture enabled, and to autograd when capture is disabled. This assumes that users won't be silly and try to use pennylane.numpy with capture enabled.

[sc-75560]

justinpickering and others added 6 commits November 11, 2024 10:15
**Context:**
See EPIC: [[P1] Update PennyLane GitHub repo README content positioning
for
researchers](https://app.shortcut.com/xanaduai/epic/76283/p1-update-content-in-pennylane-github-repository-readme?cf_workflow=500000005&ct_workflow=all&group_by=none)

---------

Co-authored-by: Josh Izaac <[email protected]>
Co-authored-by: ixfoduap <[email protected]>
**Context:**

Follow up PR addressing the `-0.01%` project code coverage issue that
slipped through the cracks in #6530.

**Description of the Change:**

Removed the relevant check because it was being exclusively tested by a
removed unit test (associated with `qml.shadows.shadow_expval`),

Here's the old test that was removed which was testing that logic
branch:
```python
...
    def test_non_shadow_error(self):
        """Test that an exception is raised when the decorated QNode does not
        return shadows"""
        dev = qml.device("default.qubit", wires=1, shots=100)

        @partial(qml.shadows.shadow_expval, H=qml.PauliZ(0))
        @qml.qnode(dev)
        def circuit():
            qml.Hadamard(0)
            return qml.expval(qml.PauliZ(0))

        msg = "Tape measurement must be ClassicalShadowMP, got 'ExpectationMP'"
        with pytest.raises(ValueError, match=msg):
            circuit()
...
```

**Benefits:** Better code coverage.

**Possible Drawbacks:** None.

[sc-77485]
Base automatically changed from plxpr-interpreter-base to master November 11, 2024 19:37
@mudit2812 mudit2812 changed the title Add TransformTrace, TransformTracer and TransformTraceInterpreter classes for transforming PLxPR Add TransformTrace, TransformTracer and TransformInterpreter classes for transforming PLxPR Nov 12, 2024
self._is_informative,
self._final_transform,
)
)
return qnode

def custom_plxpr_transform(self, fn):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just set during initialization?

I know this is how we are doing this type of syntax with custom qnode transform, but I still find that API very confusing. Keeping that confusing syntax was a heated battle I lost over the cost of change. We don't have the cost of change here, so I'd really not like to carry over bad design decisions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Chatted offline. This is a good idea, I'm changing the implementation to reflect this.

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.

What if someone wanted to implement via an interpreter-based approach rather than a trace-based approach?

@mudit2812
Copy link
Contributor Author

What if someone wanted to implement via an interpreter-based approach rather than a trace-based approach?

This is something I've thought about for some time. There are cases where we can either only build the transform using Interpreters, or cases where using Interpreters will be more efficient/easy. For now, I want to focus on transforms that can be implemented using Traces, with my reasoning being the following:

When we reworked transforms, a major factor for that was to standardize how to define transforms. If we start looking into having different implementations for transforms with program capture, that will likely cause issues down the line by making transforms harder to maintain.

Nevertheless, this is an ongoing project so we have the flexibility of making changes at a whim. If we decide later that it is fine to have multiple types of transforms, or to only have Interpreter-based transforms, then we can make that change as needed. I'm going to try to be more vigilant about issues that come up with final-style transforms and try to avoid getting married to Traces 😅 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.