add auto-configuration of threads for qasm simulator #61
Conversation
hhorii
changed the title
hhorii
changed the title
hhorii
force-pushed
the
refactor_thread
branch
from
0c87210 to
00de273
Compare
hhorii
changed the title
hhorii
force-pushed
the
refactor_thread
branch
2 times, most recently
from
9addd65 to
e37c7d0
Compare
…ization. add a config parameter to standalone qasm simulator.
There was a problem hiding this comment.
Thanks for the great job @hhorii
These are dense and complex changes, so I have many questions because I want to fully understand what's going on behind the scenes.
Once is perfectly clear, we may want to iterate again and try to make thing even simpler.
| // Set parallel_experiments_ to max_parallel_threads if necessary | ||
| parallel_experiments_ = 1; // default is 1 | ||
| if (max_parallel_experiments_ > 1 // input exists | ||
| && qobj.circuits.size() >= max_parallel_threads_ // sufficient parallelism |
There was a problem hiding this comment.
What's the problem of having less number of circuits than maximum number threads? I mean, if we have 6 circuits and 8 threads, we still want parallelism, right?
There was a problem hiding this comment.
We should use threads to update states in most case for performance. But I'm now thinking that you are right.
I guess max_parallel_experiments_ will not be configured in most cases. If a user explicitly specifies it, I guess that the user wants to prioritize experiment-level parallelism.
| parallel_experiments_ = 1; // default is 1 | ||
| if (max_parallel_experiments_ > 1 // input exists | ||
| && qobj.circuits.size() >= max_parallel_threads_ // sufficient parallelism | ||
| && max_parallel_experiments_ >= max_parallel_threads_) { // sufficient parallelism |
There was a problem hiding this comment.
Same here, what if I have 8 parallel experiments but 16 threads. We still want parallelism.
| int total_memory = 0; | ||
| for (Circuit &circ: qobj.circuits) | ||
| total_memory += state.required_memory_mb(circ.num_qubits, circ.ops); | ||
| if (total_memory <= max_statevector_memory_mb_) { |
There was a problem hiding this comment.
ok, so if we don't have enough memory for full parallelization, we just set parallelization at experiment level, isn't it?
There was a problem hiding this comment.
Right. But I'm thinking more precisely as follows.
// if memory allows, execute experiments in parallel
std::vector<int> required_memory_mb_list;
for (const Circuit &circ: circuits)
required_memory_mb_list.push_back(state.required_memory_mb(circ.num_qubits, circ.ops));
std::sort(required_memory_mb_list.begin(), required_memory_mb_list.end(), std::greater<int>());
int total_memory = 0;
parallel_experiments_ = 0;
for (int required_memory_mb: required_memory_mb_list) {
total_memory += required_memory_mb;
if (total_memory > max_statevector_memory_mb_)
break;
++parallel_experiments_;
}
if (parallel_experiments_ == 0) {
throw std::runtime_error("a circuit requires more memory than configured max_statevector_memory_mb.");
} else if (parallel_experiments_ != 1) {
if (parallel_experiments_ > max_parallel_experiments_)
parallel_experiments_ = max_parallel_experiments_;
parallel_shots_ = 1;
parallel_state_update_ = 1;
return;
}
513cf58 to
8fc8d3a
Compare
* Update QuantumError, ReadoutError to work with QuantumChannel * update tests
src/base/controller.hpp
Outdated
| if (JSON::check_key("max_memory_mb", config)) { | ||
| JSON::get_value(max_memory_mb_, "max_memory_mb", config); | ||
| } else { | ||
| auto system_memory_mb = (int) get_system_memory_mb(); |
There was a problem hiding this comment.
System memory can't be negative, why do you need a C-style cast to signed integer here?
src/base/controller.hpp
Outdated
| static_cast<int>(circ.shots) }); | ||
|
|
||
| if (max_parallel_shots_ < 1) { | ||
| // if max_parallel_shots is not configured, nested parallelization is avoided |
There was a problem hiding this comment.
The comment refers to the line above, could you move it right above the: if (max_parallel_shots_ 1) { ?
src/base/controller.hpp
Outdated
| parallel_state_update_ = max_parallel_threads_; | ||
| } | ||
| } else { | ||
| // otherwise, |
There was a problem hiding this comment.
There's no need for this comment :)
|
@hhorii just one last thing before merging. Could you please add an entry in the CHANGELOG so we can keep track very easily of what have changed in this release?? |
* Minor refactoring of Standalone exit code paths
(Windows) * Refactor some code from the Extended Stabilizer
add auto-configuration of threads for qasm simulator
Summary
This PR changes qasm simulator to use an appropriate number of threads.
Details and comments
max_statevector_memory_mbmax_statevector_memory_mb(largest power-of-two value in the system memory)