XanaduAI / QMLT

The Quantum Machine Learning Toolbox (QMLT) is a Strawberry Fields application that simplifies the optimization of variational quantum circuits (also known as parametrized quantum circuits).
https://qmlt.readthedocs.io
Apache License 2.0
114 stars 23 forks source link

Refactor in examples and helpers. #1

Closed mstechly closed 6 years ago

codecov[bot] commented 6 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@3a0f56e). Click here to learn what that means. The diff coverage is n/a.

@@           Coverage Diff           @@
##             master     #1   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?     11           
  Lines             ?    735           
  Branches          ?      0           
=======================================
  Hits              ?    735           
  Misses            ?      0           
  Partials          ?      0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3a0f56e...c204926. Read the comment docs.

co9olguy commented 6 years ago

Thanks for the PR @mstechly :) @mariaschuld @josh146 any comments?

josh146 commented 6 years ago

Looks great @mstechly! I've added some small comments (all minor), and wait for @mariaschuld to have a look.

mstechly commented 6 years ago

@mariaschuld I think that consistency between numerical and tf is important for the beginner - it was a little bit confusing to me when I wanted to try the other backend. Maybe we could add a commented section, which shows the alternative way? I know that leaving commented code everywhere is not the best practice, but these are examples and not production code, so I think it will be more instructive. WDYT?

mariaschuld commented 6 years ago

Ok, you convinced me! :) Let's not leave comments, but after the pull I will mention the old version in the doc tutorials.

Happy to merge from my side, conditioned on Joshes final comments. Think he is traveling at the moment...

josh146 commented 6 years ago

Just had a look. Perhaps the best approach is to use the same parameters list syntax in the basic tutorials, but have a extra section in the TF tutorial providing the alternative naming approach? Aside from that, looks good to merge from me.