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

[NVIDIA] Set proper math type for convolution #702

Merged

Conversation

apavliuk55
Copy link
Contributor

Details:

  • Sets proper convolution math type according to the math type of the selected algo

@apavliuk55 apavliuk55 requested a review from a team as a code owner August 8, 2023 13:19
@github-actions github-actions bot added the category: NVIDIA plugin OpenVINO NVIDIA plugin label Aug 8, 2023
@apavliuk55 apavliuk55 changed the title [NVIDIA] Set proper math type for convloution [NVIDIA] Set proper math type for convoloution Aug 8, 2023
@apavliuk55 apavliuk55 changed the title [NVIDIA] Set proper math type for convoloution [NVIDIA] Set proper math type for convolution Aug 8, 2023
@nkogteva
Copy link
Contributor

nkogteva commented Aug 8, 2023

And what about hardcoded value CUDNN_TENSOR_OP_MATH_ALLOW_CONVERSION, which you try to change previosly? Should we avoid convertion if user specify inference precision == f32?

@apavliuk55
Copy link
Contributor Author

And what about hardcoded value CUDNN_TENSOR_OP_MATH_ALLOW_CONVERSION, which you try to change previosly? Should we avoid convertion if user specify inference precision == f32?

We tried CUDNN_TENSOR_OP_MATH value but it didn't help to solve the precision issue for some tests with FP32 precision.
So we left the previous value, it won't affect FP16 precision.
For FP32 precision, I think, we should leave it as is because there were no problems with FP32 accuracy with this setting.

@nkogteva
Copy link
Contributor

nkogteva commented Aug 9, 2023

And what about hardcoded value CUDNN_TENSOR_OP_MATH_ALLOW_CONVERSION, which you try to change previosly? Should we avoid convertion if user specify inference precision == f32?

We tried CUDNN_TENSOR_OP_MATH value but it didn't help to solve the precision issue for some tests with FP32 precision. So we left the previous value, it won't affect FP16 precision. For FP32 precision, I think, we should leave it as is because there were no problems with FP32 accuracy with this setting.

It's not just about tests. If the user has explicitly specified an inference precision f32, we should avoid converting to lower precision, is not it?

@apavliuk55 apavliuk55 force-pushed the fix/proper-convolution-math-type branch from 6a5af66 to 8d74365 Compare August 9, 2023 12:57
@apavliuk55
Copy link
Contributor Author

And what about hardcoded value CUDNN_TENSOR_OP_MATH_ALLOW_CONVERSION, which you try to change previosly? Should we avoid convertion if user specify inference precision == f32?

We tried CUDNN_TENSOR_OP_MATH value but it didn't help to solve the precision issue for some tests with FP32 precision. So we left the previous value, it won't affect FP16 precision. For FP32 precision, I think, we should leave it as is because there were no problems with FP32 accuracy with this setting.

It's not just about tests. If the user has explicitly specified an inference precision f32, we should avoid converting to lower precision, is not it?

Changed to CUDNN_TENSOR_OP_MATH

@nkogteva nkogteva merged commit 274c876 into openvinotoolkit:master Aug 10, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: NVIDIA plugin OpenVINO NVIDIA plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants