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

Register: Refactor #17

Merged
merged 7 commits into from
Apr 28, 2018
Merged

Register: Refactor #17

merged 7 commits into from
Apr 28, 2018

Conversation

Roger-luo
Copy link
Member

Basics

  • AbstractRegister is no more a subtype of AbstractArray. (array interface is removed)

Default Register

  • rename ids to line_orders (address would be better?)
  • rename data to state
  • Register stores the state as an Array{T, 2} (previously we allow arbitrary Array{T, N})
  • initialization is done by methods rather than constructors: zero_state, rand_state

Packing and Focusing

@Roger-luo Roger-luo added the register bugs, discussions about register label Apr 28, 2018
@Roger-luo Roger-luo added this to the v0.1 milestone Apr 28, 2018
@Roger-luo
Copy link
Member Author

Exported APIs

  • nqubit, nbatch, line_orders, state
  • eltype, copy
  • focus!
  • zero_state, rand_state

Note: we don't export Register as an API, since user should use zero_state or rand_state to create a default register.

@coveralls
Copy link

coveralls commented Apr 28, 2018

Pull Request Test Coverage Report for Build 56

  • 57 of 59 (96.61%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.7%) to 96.774%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Register.jl 47 49 95.92%
Totals Coverage Status
Change from base Build 38: 0.7%
Covered Lines: 60
Relevant Lines: 62

💛 - Coveralls

src/Register.jl Outdated
Abstract type for quantum registers, all quantum registers supports
the interface of julia arrays.
Abstract type for quantum registers, all quantum registers contains a
subtype of `AbstractArray` as member `state`.

## Parameters
- `M` is the number of qubits
Copy link
Member

Choose a reason for hiding this comment

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

M -> N

function Register(state::Array{T, 2}) where T
len, nbatch = size(state)
ispow2(len) || throw(Compat.InexactError(:Register, Register, state))
N = log2i(len)
Copy link
Member

Choose a reason for hiding this comment

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

I think this realization of log2i is much better: https://groups.google.com/forum/#!topic/julia-users/YaACmwePGxM

src/Register.jl Outdated

pack `ids` together to the first k-dimensions.
pack `orders` together to the first k-dimensions.
Copy link
Member

Choose a reason for hiding this comment

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

name pack is too short, and not intuitive, make it longer, like change_order.

src/Register.jl Outdated

# set default type
zero_state(nqubit::Int, nbatch::Int=1) = zero_state(Complex128, nqubit, nbatch)
Copy link
Member

Choose a reason for hiding this comment

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

Keep parameters in order nqubit, nbatch, T, otherwise confusing to users.
By the way, multiple T is not needed. Just fix the datatype to complex128, otherwise, you need to cope with data type conversion (not worthy).

data::Array{T, N}
ids::Vector{Int}
mutable struct Register{N, B, T} <: AbstractRegister{N, B, T}
state::Array{T, 2}
Copy link
Member

Choose a reason for hiding this comment

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

I made a mistake, now I think rank-3 tensor makes more sense, because in this way, the array shape contains all informations about qubit and batch. otherwise, we can not distinguish remaining dimension and batch dimension without query the register.

src/Register.jl Outdated
remained_size = 2^(M-K)
data = reshape(src.data, exposed_size, remained_size)
Register(M, 1, data, src.ids)
exposed_size(orders::NTuple{K, Int}) where K = 2^K
Copy link
Member

Choose a reason for hiding this comment

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

nexposed_qubit, since you have nqubit API, it is more natural way to design API.
use 1<<n instead of 2^K! about 5 times faster!

@codecov-io
Copy link

codecov-io commented Apr 28, 2018

Codecov Report

Merging #17 into master will increase coverage by 0.65%.
The diff coverage is 96.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   96.11%   96.77%   +0.65%     
==========================================
  Files           3        3              
  Lines         103       62      -41     
==========================================
- Hits           99       60      -39     
+ Misses          4        2       -2
Impacted Files Coverage Δ
src/MathUtils.jl 100% <100%> (ø) ⬆️
src/Register.jl 96.07% <96%> (+1.34%) ⬆️

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 46dee17...71f872a. Read the comment docs.

@Roger-luo Roger-luo merged commit 1a7d01b into master Apr 28, 2018
@Roger-luo Roger-luo deleted the register branch April 28, 2018 15:40
Roger-luo added a commit that referenced this pull request Dec 7, 2021
* add benchmarks
Roger-luo pushed a commit that referenced this pull request Dec 8, 2021
GiggleLiu added a commit that referenced this pull request Apr 21, 2024
Update for new Multigraph backend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
register bugs, discussions about register
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants