catalyst icon indicating copy to clipboard operation
catalyst copied to clipboard

[Frontend] Fix decomposition tests

Open erick-xanadu opened this issue 1 year ago • 3 comments

Context: Recent changes in the TOML files broke some tests.

Description of the Change: Fix those tests.

Benefits: Latest/latest/latest passes again (at least for lightning.qubit).

Comment:

The minimum version of pennylane-lightning required is 0.37.0-dev7 but those wheels failed to be uploaded. I have used 0.37.0-dev9. If the minimum version required is required for this PR, let me know and I will ask the team to push wheels for -dev7

[sc-63376]

erick-xanadu avatar May 15 '24 13:05 erick-xanadu

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.08%. Comparing base (0dc7538) to head (02a2211). Report is 302 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #745      +/-   ##
==========================================
- Coverage   98.10%   98.08%   -0.03%     
==========================================
  Files          69       69              
  Lines        9550     9550              
  Branches      746      746              
==========================================
- Hits         9369     9367       -2     
- Misses        147      148       +1     
- Partials       34       35       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 15 '24 15:05 codecov[bot]

Hello. You may have forgotten to update the changelog! Please edit doc/changelog.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.

github-actions[bot] avatar May 15 '24 21:05 github-actions[bot]

I'm not sure how much of this PR is clashing with #718 and #712, could you double check?

dime10 avatar May 15 '24 21:05 dime10

I'm not sure how much of this PR is clashing with https://github.com/PennyLaneAI/catalyst/pull/718 and https://github.com/PennyLaneAI/catalyst/pull/712, could you double check?

I'm happy to close this PR without merging if #718 and #712 would fix latest/latest/latest.

erick-xanadu avatar May 16 '24 13:05 erick-xanadu

I'm not sure how much of this PR is clashing with #718 and #712, could you double check?

I'm happy to close this PR without merging if #718 and #712 would fix latest/latest/latest.

We need #712 . I would suggest to merge it instead of this PR. #712 addresses exactly this issue, but in my opinion - in a more systematic way: it adds an option to modify capabilities directly so we no longer need to stick to a particular toml syntax in tests.

The #718 fixes another toml leftover, it requires #712 as a prerequisite.

sergei-mironov avatar May 23 '24 10:05 sergei-mironov

I'm not sure how much of this PR is clashing with #718 and #712, could you double check?

I'm happy to close this PR without merging if #718 and #712 would fix latest/latest/latest.

We need #712 . I would suggest to merge it instead of this PR. #712 addresses exactly this issue, but in my opinion - in a more systematic way: it adds an option to modify capabilities directly so we no longer need to stick to a particular toml syntax in tests.

The #718 fixes another toml leftover, it requires #712 as a prerequisite.

Let's give those PRs the final push! @grwlf can you ensure relevant discussions are resolved with the reviewers and merged this week?

dime10 avatar May 23 '24 14:05 dime10

@dime10 and @grwlf just commenting that this is probably no longer required, right?

erick-xanadu avatar Jun 18 '24 13:06 erick-xanadu

@dime10 and @grwlf just commenting that this is probably no longer required, right?

Probably not 👍

dime10 avatar Jun 18 '24 15:06 dime10