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

Clean up Device.batch_transform to use split_non_commuting #5828

Merged
merged 40 commits into from
Jun 18, 2024

Conversation

astralcai
Copy link
Contributor

Context:
#5729 introduced the unified split_non_commuting. Now the legacy device can use split_non_commuting in all scenarios.

Description of the Change:
Cleans up batch_transform to use split_non_commuting

Benefits:
Cleaner code, prepares for the deprecation of hamiltonian_expand and sum_expand.

Related GitHub Issues:
[sc-61253]

@astralcai astralcai changed the base branch from master to split-non-com-non-pauli June 10, 2024 16:04
Base automatically changed from split-non-com-non-pauli to master June 10, 2024 19:07
@astralcai astralcai changed the base branch from master to split-non-com-diff June 11, 2024 18:25
Base automatically changed from split-non-com-diff to master June 11, 2024 22:19
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (518bcc3) to head (eaf611f).
Report is 245 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5828      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         421      421              
  Lines       40464    40203     -261     
==========================================
- Hits        40335    40073     -262     
- Misses        129      130       +1     

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

@astralcai astralcai marked this pull request as ready for review June 12, 2024 19:15
Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Nice @astralcai !
Took me some time to dig through the logic, but it is nicer now! :)
Also good to see the reduction of number of terms in some scenarios! 🎉

Had only a few quite small comments, basically ready to approve.

pennylane/_device.py Outdated Show resolved Hide resolved
pennylane/_device.py Outdated Show resolved Hide resolved
pennylane/_device.py Show resolved Hide resolved
tests/devices/default_qubit/test_default_qubit_tracking.py Outdated Show resolved Hide resolved
tests/interfaces/test_autograd.py Outdated Show resolved Hide resolved
tests/test_device.py Show resolved Hide resolved
pennylane/_device.py Outdated Show resolved Hide resolved
@astralcai astralcai requested a review from dwierichs June 13, 2024 13:29
Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Thanks for the clarifications! 👍
LGTM! 🚀

pennylane/_device.py Show resolved Hide resolved
tests/test_device.py Show resolved Hide resolved
@astralcai astralcai added this to the v0.37 milestone Jun 17, 2024
Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni left a comment

Choose a reason for hiding this comment

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

LGTM!

@astralcai astralcai enabled auto-merge (squash) June 18, 2024 18:46
@astralcai astralcai merged commit eb39402 into master Jun 18, 2024
39 checks passed
@astralcai astralcai deleted the batch-transform branch June 18, 2024 19:12
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.

3 participants