-
Notifications
You must be signed in to change notification settings - Fork 1
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
Enhance Tranformations
docs
#86
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 78.51% 80.59% +2.07%
==========================================
Files 12 12
Lines 917 912 -5
==========================================
+ Hits 720 735 +15
+ Misses 197 177 -20
☔ View full report in Codecov by Sentry. |
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.
It looks AWESOME!
Just some lil requests.
docs/src/transformations.md
Outdated
using QuacIO | ||
using CairoMakie | ||
using Tenet | ||
set_theme!(resolution=(800,400)) # hide | ||
|
||
sites = [5, 6, 14, 15, 16, 17, 24, 25, 26, 27, 28, 32, 33, 34, 35, 36, 37, 38, 39, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 61, 62, 63, 64, 65, 66, 67, 72, 73, 74, 75, 76, 83, 84, 85, 94] | ||
circuit = QuacIO.parse(joinpath(@__DIR__, "sycamore_53_10_0.qasm"), format=QuacIO.Qflex(), sites=sites) | ||
tn = TensorNetwork(circuit) | ||
transformed_tn = transform(tn, Tenet.RankSimplification) |
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.
Would you mind adding some more transformations here? I know that quimb's full_simplify
function uses AntiDiagonalGauging
, DiagonalReduction
, ColumnReduction
and RankSimplification
in that order by default.
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.
Sure!
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.
I have done that but it appears that for this specific case we obtain the same network that we would obtain with only RankSimplification
. Nevertheless, we would expect some advantage for some larger circuits when we apply all the transformations.
I think it would look better if we shorten the caption below the figures. Something like "Reference" or "Original" and "Transformed" for example. What do you think? |
Yes, I think that is a good idea! I will do that now. |
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.
Just some aesthetic requests.
Only one thing left: it looks like the plot for the |
Summary
This PR enhances the documentation of the
transform
functions. We fixed the previous text and, for the transformations that enabled it, we added a small visual example showing how that particular transformation work.Moreover, we fixed a minor problem in the
DiagonalReduction
transformation, where the COPY tensor created did not have thedual
metadata, which is necessary to display the transformed tensor network correctly.