-
Notifications
You must be signed in to change notification settings - Fork 643
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
Conversation
There was a problem hiding this 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.
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Codecov ReportAttention:
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. |
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
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:
Looking forward to having this in PennyLane, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
There was a problem hiding this 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.
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
There was a problem hiding this 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.
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
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