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

Fixing string representations in tests #1292

Merged
merged 6 commits into from
Nov 12, 2024
Merged

Conversation

PietropaoloFrisoni
Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni commented Nov 7, 2024

Context: This PR in core PennyLane slightly improves the string representations for some operators. One example is: "Adjoint(S(wires=[0]) + T(wires=[0]))" -> "Adjoint(S(0) + T(0))". Catalyst contains a few tests that check these string representations, so these tests must be updated.

Description of the Change: For the above reason, we update string representations for all failing tests.

Benefits: Catalyst works again.

Possible Drawbacks: None that I can think of.

Related GitHub Issues: None.

Related Shortcut Stories: [sc-77632]

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md on your branch 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.

Copy link
Contributor

@lillian542 lillian542 left a comment

Choose a reason for hiding this comment

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

LGTM once the T gate that slipped through is updated :)

albi3ro added a commit to PennyLaneAI/pennylane that referenced this pull request Nov 11, 2024
**PLEASE READ BEFORE MERGING**

Immediately after this PR is merged into PL, [this
one](PennyLaneAI/catalyst#1292) should be marked
as ready for review and merged into Catalyst to fix the tests.

**Context:** The string representation format of some operators is
slightly different from others. This is because some do not have their
own string representation. Furthermore, the output of the Hadamard
operator should be updated since its string representation recently
changed.

**Description of the Change:**

- Change the output in the doc of the `qml.Hadamard` operator, since its
string representation changed during the previous release
- Add a string representation for the `qml.S` operator and change the
output in the doc
- Add a string representation for the `qml.T` operator and change the
output in the doc
- Add a string representation for the `qml.SX` operator and change the
output in the doc

**Benefits:** Correct doc output and more consistency among string
representation for non-parametrized single wire operators.

**Possible Drawbacks:** This change was performed during the feature
freeze before the release of PL 0.39, but it had to be reverted because
of a single test in Catalyst that failed (the test uses the string
representation to verify the result). It is necessary to create a small
PR in Catalyst to fix such a test and link it to this story after the PL
PR is merged.

**Related GitHub Issues:** None.

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

---------

Co-authored-by: Christina Lee <[email protected]>
@paul0403
Copy link
Contributor

@joeycarter Is this the "changed-before-release-then-changed-back" string test you mentioned?

@joeycarter
Copy link
Contributor

@joeycarter Is this the "changed-before-release-then-changed-back" string test you mentioned?

Yes exactly, this PR replaces my old one here: #1263

@paul0403 paul0403 merged commit d880ab9 into main Nov 12, 2024
44 checks passed
@paul0403 paul0403 deleted the Fixing_tests_string_rep branch November 12, 2024 14:46
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.

4 participants