Skip to content

Fix/op2 as observable - #3075

Open
JerryChen97 wants to merge 26 commits into
mainfrom
fix/op2-as-observable
Open

Fix/op2 as observable#3075
JerryChen97 wants to merge 26 commits into
mainfrom
fix/op2-as-observable

Conversation

@JerryChen97

@JerryChen97 JerryChen97 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Context:
Not surprisingly, porting Y and CY into Operator2 introduced lots of failures. Here in this PR we aim to fix as much as possible.

Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:
[sc-126089]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be reverted once the fix done

@JerryChen97 JerryChen97 self-assigned this Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If this works then I just repalce inplace

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this look safe to you @paul0403 ? This PR is technically still WIP, but it seems that this file specifically blocks other work (porting Z), so I'd like to hear some early-stage feedback

1. It's mimicing PL Test, which requires lots of maintanence effort
2. With PL+Catalyst bound together strategy ahead, it's redundant

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suggest we directly delete this chunk of PL-mimicing tests. What do you think @paul0403 @kipawaa

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is needed, since we're testing catalyst.control and catalyst.adjoint rather than pennylane.control and pennylane.adjoint. Maybe I'm missing some context here though?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then I can cherry pick and keep catalyst.*? Also actually old frontend might be deprecated soon, but yes we still don't have a clear "what to burn" list yet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.28%. Comparing base (c67c55b) to head (acf26ce).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3075      +/-   ##
==========================================
+ Coverage   96.92%   97.28%   +0.35%     
==========================================
  Files         166       40     -126     
  Lines       19133     4935   -14198     
  Branches     1815        0    -1815     
==========================================
- Hits        18545     4801   -13744     
+ Misses        429      134     -295     
+ Partials      159        0     -159     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JerryChen97
JerryChen97 requested a review from kipawaa July 31, 2026 17:16
Comment thread .github/workflows/check-catalyst.yaml Outdated
Comment thread .github/workflows/check-catalyst.yaml Outdated
Comment thread .github/workflows/check-catalyst.yaml Outdated
zczccz already merged

Co-authored-by: Yushao Chen (Jerry) <chenys13@outlook.com>
@JerryChen97

Copy link
Copy Markdown
Contributor Author

So the three template failures were covered by (at least) two different things: 1. the adjoint isse 2. the dynamic ctrl value issue

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.

2 participants