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

Implement FABLE as a Template (issue #4848) #5107

Merged
merged 95 commits into from
Apr 11, 2024

Conversation

austingmhuang
Copy link
Contributor

@austingmhuang austingmhuang commented Jan 25, 2024

Context: Implemented a new template for issue 4848

Description of the Change: Implements FABLE as a template to efficiently encodes the block matrix.

Benefits:

Possible Drawbacks:

Related GitHub Issues:
#4848

Copy link
Contributor

@soranjh soranjh left a comment

Choose a reason for hiding this comment

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

Thanks @austingmhuang, the PR already looks good. I left some comments and suggestions. If you have any questions about the comments or want clarification, ask here and I will reply.

.gitignore Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
tests/templates/test_subroutines/test_fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
@Jaybsoni Jaybsoni self-requested a review January 31, 2024 15:32
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (5cfdedd) 99.36% compared to head (fab4792) 99.67%.
Report is 15 commits behind head on master.

Files Patch % Lines
pennylane/templates/subroutines/fable.py 90.62% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5107      +/-   ##
==========================================
+ Coverage   99.36%   99.67%   +0.30%     
==========================================
  Files         392      395       +3     
  Lines       35850    35774      -76     
==========================================
+ Hits        35623    35656      +33     
+ Misses        227      118     -109     

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

@Jaybsoni
Copy link
Contributor

Jaybsoni commented Feb 6, 2024

Hi @austingmhuang, great work so far! There are just some failures in the CI to fix and some comments from reviewers to address and it should be in good condition to merge into PennyLane! Could please do the following:

  • We have formatting tests which check that any new code committed to Pennylane is standardized under some conventions. You can automatically fix these formatting issues by pip installing the contents of requirements-dev.txt, and running the command make format in your terminal inside the pennyLane repo.
  • CodeCov, is a CI tool responsible for ensuring that every line of code is tested to some extent. This ensures we aren't committing un-tested code which might introduce bugs. Could you please add test cases for the lines of code highlighted by Codecov. You can find these lines annotated under the "Files changed" heading here on GitHub.

Looking forward to having this in PennyLane,
Cheers,

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.66%. Comparing base (1089bb3) to head (245a6f5).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5107      +/-   ##
==========================================
- Coverage   99.66%   99.66%   -0.01%     
==========================================
  Files         402      404       +2     
  Lines       37635    37554      -81     
==========================================
- Hits        37510    37428      -82     
- Misses        125      126       +1     

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

pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
tests/templates/test_subroutines/test_fable.py Outdated Show resolved Hide resolved
tests/templates/test_subroutines/test_fable.py Outdated Show resolved Hide resolved
@soranjh soranjh requested a review from trbromley April 10, 2024 14:16
austingmhuang and others added 3 commits April 10, 2024 10:17
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @austingmhuang, no major blockers but a few more comments.

pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
austingmhuang and others added 4 commits April 10, 2024 15:43
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
Copy link
Contributor

@soranjh soranjh left a comment

Choose a reason for hiding this comment

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

Great work @austingmhuang! Please consider adding a few final minor corrections.

pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/fable.py Outdated Show resolved Hide resolved
@austingmhuang austingmhuang merged commit 803139d into PennyLaneAI:master Apr 11, 2024
38 checks passed
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.

None yet

5 participants