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

Update onednn #15632

Closed
wants to merge 1 commit into from
Closed

Conversation

RICKIE777
Copy link
Contributor

Details:

  • Delete 3D depthwise unimplemented judgments

@dmitry-gorokhov
Copy link
Contributor

@EgorDuplensky We have to options here: I can direcly merge this above oneDNN 2.7 or you can include this change into your oneDNN 3.0 migration PR. Up to you.

@EgorDuplensky
Copy link
Contributor

@EgorDuplensky We have to options here: I can direcly merge this above oneDNN 2.7 or you can include this change into your oneDNN 3.0 migration PR. Up to you.

I will correct this on oneDNN 3.0 migration branch by fixing the original commit then.

@EgorDuplensky
Copy link
Contributor

EgorDuplensky commented Feb 13, 2023

@EgorDuplensky We have to options here: I can direcly merge this above oneDNN 2.7 or you can include this change into your oneDNN 3.0 migration PR. Up to you.

I will correct this on oneDNN 3.0 migration branch by fixing the original commit then.

@dmitry-gorokhov Done
@RICKIE777 Thank you for the change, we will apply it to the original commit, since we are currently performing the rebase anyway

@EgorDuplensky
Copy link
Contributor

@dmitry-gorokhov @RICKIE777
One more thing. It would be good to have a test case for the issue we found.
Since we are going to drop this PR, it can be transformed into the PR with a test case, which will currently fail, but will pass after the PR with the test will be rebased onto oneDNN migration.

@RICKIE777
Copy link
Contributor Author

@dmitry-gorokhov @RICKIE777 One more thing. It would be good to have a test case for the issue we found. Since we are going to drop this PR, it can be transformed into the PR with a test case, which will currently fail, but will pass after the PR with the test will be rebased onto oneDNN migration.

Thanks @dmitry-gorokhov and @EgorDuplensky.
Maybe this model in JIRA CVS-98185 fits the need.
In Xeon machine, if this PR is not merged, the kernel selection is jit_uni_int8:avx2. After merging the PR, the kernel selection should be jit_int8:avx512_core_vnni.

@EgorDuplensky
Copy link
Contributor

EgorDuplensky commented Feb 15, 2023

@dmitry-gorokhov @RICKIE777 One more thing. It would be good to have a test case for the issue we found. Since we are going to drop this PR, it can be transformed into the PR with a test case, which will currently fail, but will pass after the PR with the test will be rebased onto oneDNN migration.

Thanks @dmitry-gorokhov and @EgorDuplensky. Maybe this model in JIRA CVS-98185 fits the need. In Xeon machine, if this PR is not merged, the kernel selection is jit_uni_int8:avx2. After merging the PR, the kernel selection should be jit_int8:avx512_core_vnni.

The idea is to add a single_layer_test instance which reproduces the issue you observe with the model.
So the next time we occasionally remove this peace of code again the test will fail

@RICKIE777
Copy link
Contributor Author

@dmitry-gorokhov @RICKIE777 One more thing. It would be good to have a test case for the issue we found. Since we are going to drop this PR, it can be transformed into the PR with a test case, which will currently fail, but will pass after the PR with the test will be rebased onto oneDNN migration.

Thanks @dmitry-gorokhov and @EgorDuplensky. Maybe this model in JIRA CVS-98185 fits the need. In Xeon machine, if this PR is not merged, the kernel selection is jit_uni_int8:avx2. After merging the PR, the kernel selection should be jit_int8:avx512_core_vnni.

The idea is to add a single_layer_test instance which reproduces the issue you observe with the model. So the next time we occasionally remove this peace of code again the test will fail

Thanks @EgorDuplensky. I have a question about it. The kernel is jit_int8:avx512_core_vnni. The modified file is jit_avx512_core_x8s8s32_conv_kernel.cpp.
In which file can I add test cases for quantized int8? The test cases in the single_layer_test/convolution.cpp file are test float data.

@RICKIE777
Copy link
Contributor Author

The modification has been merged into onednn v3.0_for_ie_master_migration. The onednn team has not yet merged int8 single-layer tests. For the test case of this PR, I have submitted a ticket https://jira.devtools.intel.com/browse/MFDNN-9592. So this PR can be closed.

@RICKIE777 RICKIE777 closed this Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants