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

Enhance Tranformations docs #86

Merged
merged 11 commits into from
Sep 13, 2023
Merged

Conversation

jofrevalles
Copy link
Member

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 the dual metadata, which is necessary to display the transformed tensor network correctly.

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +2.07% 🎉

Comparison is base (42c4e58) 78.51% compared to head (5172bf4) 80.59%.
Report is 11 commits behind head on master.

❗ Current head 5172bf4 differs from pull request most recent head 6bcd695. Consider uploading reports for the commit 6bcd695 to get more accurate results

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     
Files Changed Coverage Δ
src/Transformations.jl 97.50% <100.00%> (+0.03%) ⬆️

... and 3 files with indirect coverage changes

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

Copy link
Member

@mofeing mofeing left a 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.

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)
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!

Copy link
Member Author

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.

docs/src/transformations.md Outdated Show resolved Hide resolved
@mofeing
Copy link
Member

mofeing commented Sep 13, 2023

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?

@jofrevalles
Copy link
Member Author

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.

@mofeing mofeing changed the title Enhance Tranformations.jl docs Enhance Tranformations docs Sep 13, 2023
@mofeing mofeing added this to the 0.3 milestone Sep 13, 2023
@mofeing mofeing added the documentation Improvements or additions to documentation label Sep 13, 2023
Copy link
Member

@mofeing mofeing left a 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.

src/Transformations.jl Outdated Show resolved Hide resolved
src/Transformations.jl Outdated Show resolved Hide resolved
@jofrevalles jofrevalles requested a review from mofeing September 13, 2023 13:42
@mofeing
Copy link
Member

mofeing commented Sep 13, 2023

Only one thing left: it looks like the plot for the DiagonalReduction is above the docstring instead of below. Would you mind correcting that please?

@mofeing mofeing merged commit 5edc49d into master Sep 13, 2023
2 of 3 checks passed
@mofeing mofeing deleted the feature/enhance-transformation-docs branch September 13, 2023 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants