Skip to content

Fix for GPU batched optimization with Pauli noise#1482

Merged
hhorii merged 19 commits into
Qiskit:mainfrom
doichanj:pauli_noise_fix
Mar 29, 2022
Merged

Fix for GPU batched optimization with Pauli noise#1482
hhorii merged 19 commits into
Qiskit:mainfrom
doichanj:pauli_noise_fix

Conversation

@doichanj
Copy link
Copy Markdown
Collaborator

Summary

This PR is fix for issue #1473

Details and comments

Added option to allocate multiple threads per GPU and nested parallelism is applied to parallelize runtime noise sampling for Pauli noise in batched optimization.

@doichanj doichanj requested a review from hhorii March 15, 2022 02:26
@hhorii hhorii added the stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable label Mar 15, 2022
Copy link
Copy Markdown
Collaborator

@hhorii hhorii left a comment

Choose a reason for hiding this comment

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

I think NoiseModel should not have a new method pauli_only(). StateChunk<state_t>::apply_ops_multi_shots() can efficiently check whether all the quantum errors consist of pauli gates only by iterating NoiseMode.opset().

Comment thread src/noise/quantum_error.hpp Outdated
}


bool QuantumError::pauli_only(void) const
Copy link
Copy Markdown
Collaborator

@hhorii hhorii Mar 28, 2022

Choose a reason for hiding this comment

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

I believe that this method is not necessary.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment thread src/noise/noise_model.hpp Outdated
model = NoiseModel(js);
}

bool NoiseModel::pauli_only(void) const
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This method name is odd because NoiseModel may have another noise as read out errors.

I think callers can check gates are only pauli by checking NoiseModel.opset().

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I recover the prior code to check if sampled gates only contains Pauli gates or not.

Comment thread src/simulators/state_chunk.hpp Outdated
uint_t rng_seed,
bool final_ops);
bool final_ops,
bool pauli_only);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we rename pauli_only with batched_pauli_ops?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment on lines +255 to +259
* ``num_threads_per_group`` (int): This option sets the number of
threads per group. For GPU simulation, this value sets number of
threads per GPU. This parameter is used to optimize Pauli noise
simulation with multiple-GPUs (Default: 1).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

group sounds too general. Can we rename this with num_threads_per_device?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

changed to num_threads_per_device

Comment thread src/simulators/state_chunk.hpp Outdated
int_t i;
int_t i_begin,n_shots;

bool pauli_only = noise.pauli_only();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe that pauli_only should be renamed with batched_pauli_ops.
Also, it is better to check NoiseModel.opset() here instead of adding a new method pauli_only().

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed

Copy link
Copy Markdown
Collaborator

@hhorii hhorii left a comment

Choose a reason for hiding this comment

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

LGTM. In my understanding, this change does not need tests because this improves performance only by parallelizing noise sampling.

@hhorii hhorii merged commit ce6c733 into Qiskit:main Mar 29, 2022
@hhorii hhorii added this to the Aer 0.10.4 milestone Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants