Skip to content
This repository has been archived by the owner on Dec 18, 2021. It is now read-only.

add multi threading #19

Merged
merged 9 commits into from
Apr 24, 2020
Merged

add multi threading #19

merged 9 commits into from
Apr 24, 2020

Conversation

Roger-luo
Copy link
Member

Add multi threading support. Pending for benchmark results.

@Roger-luo
Copy link
Member Author

I think controldo can use multi threading as well, but I'll check this later.

@codecov
Copy link

codecov bot commented May 22, 2019

Codecov Report

Merging #19 into master will decrease coverage by 0.01%.
The diff coverage is 89.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage   77.18%   77.16%   -0.02%     
==========================================
  Files           8        8              
  Lines         434      438       +4     
==========================================
+ Hits          335      338       +3     
- Misses         99      100       +1
Impacted Files Coverage Δ
src/instruct.jl 92.5% <89.47%> (-0.45%) ⬇️

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 2f785b6...69e218a. Read the comment docs.

@Roger-luo
Copy link
Member Author

Roger-luo commented May 23, 2019

@Roger-luo
Copy link
Member Author

Roger-luo commented May 23, 2019

need to add a threshold to disable multi threading when the number of qubits is not large.

Or should we just let user to decide this? since user can always close it by tweaking JULIA_NUM_THREADS @GiggleLiu

@Roger-luo
Copy link
Member Author

OK so it looks like the overhead of multi threads, should define a threshold to avoid them.

https://gist.github.com/Roger-luo/26682142d70f2d24c8a8bea36518900f

@GiggleLiu
Copy link
Member

need to add a threshold to disable multi threading when the number of qubits is not large.

Or should we just let user to decide this? since user can always close it by tweaking JULIA_NUM_THREADS @GiggleLiu

Cool, it is better to let user decide this.

@GiggleLiu
Copy link
Member

Please show the number of threads. It looks like always a lot of overhead. Lol

@Roger-luo
Copy link
Member Author

It's in the benchmark report @GiggleLiu

@GiggleLiu
Copy link
Member

please check the GPU implementation for some inspiration of multi-threading. controldo can be parallelized.

https://github.com/QuantumBFS/CuYao.jl

@codecov
Copy link

codecov bot commented May 23, 2019

Codecov Report

Merging #19 into master will decrease coverage by 1.35%.
The diff coverage is 77.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
- Coverage   80.82%   79.47%   -1.36%     
==========================================
  Files           8        8              
  Lines         558      570      +12     
==========================================
+ Hits          451      453       +2     
- Misses        107      117      +10     
Impacted Files Coverage Δ
src/instruct.jl 90.99% <77.35%> (-4.49%) ⬇️

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 932c243...2898e0a. Read the comment docs.

@Roger-luo
Copy link
Member Author

seems to have memory leak on Linux (works fine on Mac for me). Might related to JuliaLang/julia#31923

We need to wait for this issue to be fixed I think.

@Roger-luo Roger-luo added the pending pending issues, will go into next release label May 26, 2019
@Roger-luo
Copy link
Member Author

I think it is fixed on master now: JuliaLang/julia#32217

@Roger-luo
Copy link
Member Author

we need more benchmark reports before we ship this feature.

@GiggleLiu
Copy link
Member

Any new benchmark reports?

@Roger-luo
Copy link
Member Author

Not yet. I'll have time on this after JuliaCon

@Roger-luo
Copy link
Member Author

I think if we merge this (after 1.3 is out), but we might need to ask our users to use julia 1.3 for best performance. (I'm not sure if thee new multi-threading support will be backported)

@Roger-luo
Copy link
Member Author

Roger-luo commented Aug 12, 2019

Some updates on this. The @threads macro is compatible since 1.0, thus we can have this for all 1.x, but the performance might be different. I'll encourage people to use latest version if you need multi-threading. I'm still not sure why there's some regression for single thread benchmark on this yet.

I suspect we have some cache line problem when enabling multi-threading, the scaling is not good.

@Roger-luo
Copy link
Member Author

Roger-luo commented Aug 30, 2019

We need to refactor this branch after #30 @GiggleLiu and I need to explore some more benchmark on cache line problem. Hopefully we can deliver this feature this year.

@Roger-luo
Copy link
Member Author

@GiggleLiu This looks good now. I think we could merge this first. I'll make a few following PR to fix some performance issue later.

@Roger-luo Roger-luo merged commit 656516e into master Apr 24, 2020
@Roger-luo Roger-luo deleted the multithreading branch April 24, 2020 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending pending issues, will go into next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants