[NHWC] Enable batch norm by refactoring OCL kernel #1244

ce1adon · 2021-10-28T00:51:31Z

No description provided.

atamazov · 2021-10-28T09:42:38Z

@DrizztDoUrden Please review problem description changes.

atamazov

Some cosmetic changes + recommendations

UPDATE: "Cosmetic" changes are actually mandatory.

src/solver/batchnorm/forward_spatial_single.cpp

src/kernels/MIOpenBatchNormBwdSpatial.cl

atamazov · 2021-10-28T12:18:40Z

@ce1adon What is your plan about testing?

ce1adon · 2021-10-28T16:31:33Z

@ce1adon What is your plan about testing?

As mentioned in the meeting, we will collaborate with the clients about the test.

atamazov · 2021-10-28T23:52:11Z

@ce1adon It seems like I missed that. Let's discuss next Tuesday.

cc @junliume @JehandadKhan

JehandadKhan · 2021-10-29T15:53:06Z

@ce1adon I think we need to add at least some tests to exercise this kernel.

junliume · 2021-10-29T16:07:05Z

@ce1adon I think we need to add at least some tests to exercise this kernel.

+1
check this comment out from SWDEV-293294-comment

src/kernels/MIOpenBatchNormBwdSpatial.cl

src/kernels/MIOpenBatchNormFwdTrainSpatial.cl

junliume · 2021-11-12T01:23:21Z

ping @ce1adon for updates on this PR. Thanks!

ce1adon · 2021-11-12T18:12:07Z

ping @ce1adon for updates on this PR. Thanks!

Debugging accuracy issues in ctest.

Review changes adopted

Please re-review :)

atamazov

No objections!

src/kernels/MIOpenBatchNormBwdSpatial.cl

atamazov

LGTM

atamazov · 2021-12-16T15:30:39Z

src/kernels/MIOpenBatchNormBwdSpatial.cl

+#endif
+
+#if (MIO_LAYOUT_NHWC != 0) || (MIO_LAYOUT_NHWC != 1)
+#error MIO_LAYOUT_NHWC must be 0 or 1


[Recommendation] Oops, I am afraid we need double quotes around error message.

Suggested change

#error MIO_LAYOUT_NHWC must be 0 or 1

#error "MIO_LAYOUT_NHWC must be 0 or 1"

@atamazov I'm afraid I might have messed up something and trying to correct them:

[2021-12-16T21:18:03.373Z] 13/93 Test #3: test_bn_3d_spatial_test ...............................***Failed Error regular expression found in output. Regex=[FAILED] 1.34 sec [2021-12-16T21:18:03.373Z] Choosing smaller input values for low dims [2021-12-16T21:18:03.373Z] MIOpen(OpenCL): Error [BuildProgram] Build log: /tmp/comgr-547839/input/CompileSource:446:2: error: "MIO_LAYOUT_NHWC must be 0 or 1" [2021-12-16T21:18:03.373Z] #error "MIO_LAYOUT_NHWC must be 0 or 1" [2021-12-16T21:18:03.373Z] ^ [2021-12-16T21:18:03.373Z] 1 error generated. [2021-12-16T21:18:03.373Z] Error: Failed to compile opencl source (from CL or HIP source to LLVM IR).

~~Maybe #error macro is not accepted in COMGR or hipRTC?~~
Nope, I've seen other examples in other .cl files

OMG we shall use &&, not || 🤕

@atamazov ah, I'm so distracted and should have figured it out sooner :) I'm going to change it really soon.

atamazov · 2021-12-17T16:16:08Z

@junliume

Error fix f85cedd

👍 👍 👍

atamazov

LGTM

atamazov · 2021-12-17T22:24:31Z

@ce1adon If would be nice if you bless this PR as well ;)

atamazov · 2021-12-18T23:26:39Z

@junliume The CI failure at run 19 was due to #1351

refactor ocl kernels for batchnorm nhwc

f02b6c3

ce1adon requested review from junliume and atamazov October 28, 2021 00:51

This comment has been minimized.

Sign in to view

junliume added this to the ROCm 5.0 milestone Oct 28, 2021

junliume added complexity_middle enhancement urgency_high labels Oct 28, 2021

junliume changed the title ~~refactor ocl kernels for batchnorm nhwc~~ [NHWC] Enable batch norm by refactoring OCL kernel Oct 28, 2021

atamazov requested a review from DrizztDoUrden October 28, 2021 09:42

atamazov previously requested changes Oct 28, 2021

View reviewed changes

DrizztDoUrden previously requested changes Nov 8, 2021

View reviewed changes

src/kernels/MIOpenBatchNormBwdSpatial.cl Outdated Show resolved Hide resolved

src/kernels/MIOpenBatchNormFwdTrainSpatial.cl Outdated Show resolved Hide resolved

junliume modified the milestones: ROCm 5.0, ROCm 5.1 Nov 18, 2021

ce1adon and others added 3 commits November 18, 2021 18:16

add batch norm spatial nhwc test

0b24942

Merge branch 'develop' into bn_nhwc

873353c

Fix Tidy Issue

0cd8e89

junliume requested review from DrizztDoUrden and atamazov December 6, 2021 23:05

junliume added 2 commits December 13, 2021 14:16

Merge branch 'develop' into bn_nhwc

f3a262a

Fix tidy and review requests

8285fe8

atamazov previously approved these changes Dec 15, 2021

View reviewed changes

src/kernels/MIOpenBatchNormBwdSpatial.cl Outdated Show resolved Hide resolved

Fix error and review requests

cd19418

junliume dismissed atamazov’s stale review via cd19418 December 16, 2021 00:46

muralinr mentioned this pull request Dec 16, 2021

Revert #1046 and fix BN changes introduced in #916 (real fix for SWDEV-293780 and part 1/2 of the real fix for SWDEV-295434) #1047

Open

7 tasks

atamazov previously approved these changes Dec 16, 2021

View reviewed changes

bugfix: define macro in branch results in error

e4a74f4

junliume dismissed atamazov’s stale review via e4a74f4 December 16, 2021 17:45

junliume added 4 commits December 16, 2021 12:41

fix ocl error macro

a2b1ada

Fix error on macro conditions

2043ad2

Error fix regarding format

3a0735b

Error fix

f85cedd

atamazov approved these changes Dec 17, 2021

View reviewed changes

Merge branch 'develop' into bn_nhwc

0581efa

junliume added the TESTING_CI_PASSED label Dec 23, 2021

junliume approved these changes Dec 23, 2021

View reviewed changes

junliume merged commit adcef3b into develop Dec 23, 2021

junliume deleted the bn_nhwc branch April 14, 2022 16:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NHWC] Enable batch norm by refactoring OCL kernel #1244

[NHWC] Enable batch norm by refactoring OCL kernel #1244

ce1adon commented Oct 28, 2021

This comment has been minimized.

atamazov commented Oct 28, 2021

atamazov left a comment •

edited

Loading

atamazov commented Oct 28, 2021

ce1adon commented Oct 28, 2021

atamazov commented Oct 28, 2021

JehandadKhan commented Oct 29, 2021

junliume commented Oct 29, 2021

junliume commented Nov 12, 2021

ce1adon commented Nov 12, 2021

atamazov left a comment

atamazov left a comment

atamazov Dec 16, 2021

junliume Dec 16, 2021

junliume Dec 16, 2021 •

edited

Loading

atamazov Dec 16, 2021

junliume Dec 16, 2021

atamazov commented Dec 17, 2021

atamazov left a comment

atamazov commented Dec 17, 2021

atamazov commented Dec 18, 2021

	#error MIO_LAYOUT_NHWC must be 0 or 1
	#error "MIO_LAYOUT_NHWC must be 0 or 1"

[NHWC] Enable batch norm by refactoring OCL kernel #1244

[NHWC] Enable batch norm by refactoring OCL kernel #1244

Conversation

ce1adon commented Oct 28, 2021

This comment has been minimized.

atamazov commented Oct 28, 2021

atamazov left a comment • edited Loading

Choose a reason for hiding this comment

atamazov commented Oct 28, 2021

ce1adon commented Oct 28, 2021

atamazov commented Oct 28, 2021

JehandadKhan commented Oct 29, 2021

junliume commented Oct 29, 2021

junliume commented Nov 12, 2021

ce1adon commented Nov 12, 2021

atamazov left a comment

Choose a reason for hiding this comment

atamazov left a comment

Choose a reason for hiding this comment

atamazov Dec 16, 2021

Choose a reason for hiding this comment

junliume Dec 16, 2021

Choose a reason for hiding this comment

junliume Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

atamazov Dec 16, 2021

Choose a reason for hiding this comment

junliume Dec 16, 2021

Choose a reason for hiding this comment

atamazov commented Dec 17, 2021

atamazov left a comment

Choose a reason for hiding this comment

atamazov commented Dec 17, 2021

atamazov commented Dec 18, 2021

atamazov left a comment •

edited

Loading

junliume Dec 16, 2021 •

edited

Loading