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

Create YaoTreeToPastaQList.jl #1

Merged
merged 22 commits into from
May 19, 2021
Merged

Create YaoTreeToPastaQList.jl #1

merged 22 commits into from
May 19, 2021

Conversation

VarLad
Copy link
Collaborator

@VarLad VarLad commented May 18, 2021

Covered most of the gates, except for
iSWAP, Rn, sqrt(X), CRn, and sqrt(SWAP) gates

@VarLad VarLad marked this pull request as draft May 18, 2021 23:47
@Roger-luo
Copy link
Member

we use snake case for file names except the module file. for this package, it should be simple enough to just put code inside src/YaoPastaQ.jl

1 similar comment
@Roger-luo
Copy link
Member

we use snake case for file names except the module file. for this package, it should be simple enough to just put code inside src/YaoPastaQ.jl

@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #1 (500f8ed) into master (45de326) will increase coverage by 78.94%.
The diff coverage is 78.94%.

❗ Current head 500f8ed differs from pull request most recent head 66fbb73. Consider uploading reports for the commit 66fbb73 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master       #1       +/-   ##
===========================================
+ Coverage        0   78.94%   +78.94%     
===========================================
  Files           0        1        +1     
  Lines           0       38       +38     
===========================================
+ Hits            0       30       +30     
- Misses          0        8        +8     
Impacted Files Coverage Δ
src/YaoPastaQ.jl 78.94% <78.94%> (ø)

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 45de326...66fbb73. Read the comment docs.

@VarLad
Copy link
Collaborator Author

VarLad commented May 19, 2021

@Roger-luo thats most of the gates
Can't find the equivalent for
iSWAP, Rn, sqrt(X), CRn, and sqrt(SWAP)

@VarLad VarLad marked this pull request as ready for review May 19, 2021 13:19
Copy link
Member

@Roger-luo Roger-luo left a comment

Choose a reason for hiding this comment

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

this is still far from finish, you still need:

  1. make a PastaQ regsiter
  2. write tests to get over 90% test coverage

Manifest.toml Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
src/YaoPastaQ.jl Outdated Show resolved Hide resolved
src/YaoPastaQ.jl Outdated Show resolved Hide resolved
src/YaoPastaQ.jl Outdated Show resolved Hide resolved
@VarLad
Copy link
Collaborator Author

VarLad commented May 19, 2021

@Roger-luo Okay, kindly review it once more. I'll start writing the tests if its okay this far

@Roger-luo
Copy link
Member

@VarLad you didn't implement the register type.

please do a git rm Manifest.toml to remove Manifest.toml file from this PR

you are not following the style guide, code inside module shouldn't have indent

@Roger-luo
Copy link
Member

I'll start writing the tests if its okay this far

please add tests first before asking for review

@VarLad
Copy link
Collaborator Author

VarLad commented May 19, 2021

you are not following the style guide, code inside module shouldn't have indent

Sorry, I saw this 😅

General Formatting Guidelines for Julia code contributions

    4 spaces per indentation level, no tabs

https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#general-formatting-guidelines-for-julia-code-contributions

I'll remove the indentation

@VarLad VarLad requested a review from Roger-luo May 19, 2021 18:13
@VarLad
Copy link
Collaborator Author

VarLad commented May 19, 2021

@Roger-luo If this is all correct, I guess what remains is to write the register.....

Project.toml Outdated Show resolved Hide resolved
@Roger-luo
Copy link
Member

looks good in general, we can merge this PR after change the above two coments. then you can open a new PR for the register

@VarLad
Copy link
Collaborator Author

VarLad commented May 19, 2021

Okay, now the registers....

@VarLad VarLad requested a review from Roger-luo May 19, 2021 18:34
@Roger-luo Roger-luo merged commit 2e5ec5f into QuantumBFS:master May 19, 2021
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.

2 participants