[Relay][Training] Add gradient for Crossentropy by MarisaKirisame · Pull Request #3925 · apache/tvm

MarisaKirisame · 2019-09-09T22:52:56Z

@vinx13 @junrushao1994 @SWu can you guys help review?

vinx13

LGTM except some minor

python/tvm/relay/testing/__init__.py

vinx13 · 2019-09-09T23:28:37Z

python/tvm/relay/op/nn/_nn.py

 reg.register_pattern("nn.bitserial_dense", reg.OpPattern.OUT_ELEMWISE_FUSABLE)
+
+
+


nit: remove extra blank line (only two are needed)

MarisaKirisame · 2019-09-09T23:54:51Z

@vinx13 I had address your comment. can you review again?

junrushao

lgtm

SWu · 2019-09-10T14:10:43Z

python/tvm/relay/op/nn/nn.py

                                 pack_dtype, out_dtype, unipolar)
+
+
+def cross_entropy(predictions, targets):


This should have a docstring. You should also mention that this is cross-entropy without softmax, as many frameworks equate cross-entropy to cross-entropy from logits

@MarisaKirisame can you react on @SWu 's comment (also put it in REGISTER_RELAY_OP section)

MarisaKirisame · 2019-09-10T19:53:47Z

@vinx13 @tqchen what should it's schedule be?

weberlo · 2019-09-10T19:49:09Z

src/relay/op/nn/nn.cc

+
+
+RELAY_REGISTER_OP("nn.cross_entropy")
+.describe(R"code(Computes cross entropy given preditions and targets.)code" TVM_ADD_FILELINE)


Suggested change

.describe(R"code(Computes cross entropy given preditions and targets.)code" TVM_ADD_FILELINE)

.describe(R"code(Computes cross entropy given predictions and targets.)code" TVM_ADD_FILELINE)

weberlo · 2019-09-10T19:53:31Z

src/relay/op/nn/nn.cc

+    << "y shape=" << y->shape;
+  CHECK(reporter->AssertEQ(x->shape[1], y->shape[1]))
+    << "CrossEntropy: shapes of x and y is inconsistent, "
+    << "x shape=, " << x->shape


Suggested change

<< "x shape=, " << x->shape

<< "x shape = " << x->shape << ", "

and can this be done for all of the above instances?

weberlo · 2019-09-10T19:55:15Z

python/tvm/relay/op/nn/_nn.py

+@reg.register_compute("nn.cross_entropy")
+def compute_cross_entropy(attrs, inputs, out_dtype, target):
+    x, y = inputs
+    return [-topi.sum(topi.log(x) * y / x.shape[0])]


Would it be more efficient and numerically stable to divide by the batch size after the sum?

vinx13 · 2019-09-15T21:37:57Z

@MarisaKirisame The schedule should be injective, can you check if the CUDA schedule are properly called?

MarisaKirisame · 2019-09-18T22:56:39Z

@vinx13 how can I do that? I am not really familiar with tvm low level internal.

vinx13 · 2019-09-19T00:44:27Z

@MarisaKirisame I will take a look

vinx13 · 2019-09-19T02:39:13Z

@MarisaKirisame I guess this is caused by the use of reference. It makes fusion and scheduling difficult. But I didn't reproduce the error on master, can you try rebasing?

vinx13 · 2019-09-19T04:32:54Z

python/tvm/relay/op/nn/_nn.py

 reg.register_pattern("nn.bitserial_dense", reg.OpPattern.OUT_ELEMWISE_FUSABLE)
+
+
+reg.register_schedule("nn.cross_entropy", schedule_injective)


the schedule actually should be schedule_reduce (in relay.op._reduce)

vinx13 · 2019-09-19T04:33:15Z

tests/python/relay/test_op_grad_level10.py

+from tvm.relay.testing import check_grad
+
+
+def test_crossentropy_grad():


nit: test_cross_entropy_grad

MarisaKirisame · 2019-09-19T18:30:18Z

@vinx13 @tqchen it is still the same error. as I dont has a cuda I cannot reproduce.

vinx13 · 2019-09-19T18:31:18Z

@MarisaKirisame see my comment, the schedule should be reduce

vinx13 · 2019-09-28T16:53:03Z

ping @MarisaKirisame

MarisaKirisame · 2019-10-01T18:45:41Z

@vinx13 sorry, I was pushing training on a private branch. I had addressed the issues.

MarisaKirisame · 2019-10-02T17:43:22Z

@vinx13 @tqchen can anyone take a look? I didnt change resize in any way. this is blocking training.

vinx13 · 2019-10-03T02:08:48Z

@MarisaKirisame might be a flaky case, can you restart the ci?

MarisaKirisame · 2019-10-03T04:51:45Z

@vinx13 I will restart it right now. Just FYI I also got the same error last time.

save redo max test save address comment fix

vinx13 · 2019-10-03T05:32:04Z

@MarisaKirisame you can try increasing rtol of the failing test

MarisaKirisame · 2019-10-03T17:01:31Z

@vinx13 it now work.

vinx13 · 2019-10-04T17:40:24Z

@MarisaKirisame https://github.com/dmlc/tvm/pull/3925/files#r330840227

MarisaKirisame · 2019-10-04T23:43:37Z

@vinx13 I had acted on the comment.

* save save redo max test save address comment fix * address comment * increase rtol * address review comment

* master: (21 commits) [Fix][VM] Fix VM invoke with set_params (apache#4079) [QNN] Refactor fixed point multiplication in requantize (apache#4073) Fix match case in Python-side expr functor (apache#4037) Hide symbols from dependent libraries if HIDE_PRIVATE_SYMBOLS is ON. (apache#4041) Add gradient for log-softmax (apache#4069) [DOC] Fix typos in tutorials (apache#4066) dicrease the complexity of CalcDep from exponential to linear (apache#4053) [Relay][AlterOp] Minor refactor. (apache#4064) [Relay][AlterOp] Improving support for broadcast layout alteration. (apache#4040) Add parses support for zeros_like tflite operator (apache#4042) [Bugfix][TF] reset graph after getting tag of savedmodel (apache#4055) [Relay][VM] Add more passes to VMCompiler (apache#4058) [Relay][VM] Add autotvm context when compile (apache#4062) [Bugfix] Fix target host for vm compiler (apache#4057) [Relay][Training] Add gradient for Crossentropy (apache#3925) [llvm] switch to use Align for llvm trunk (apache#4051) [Relay][TopHub] Add switch to disable TopHub download (apache#4015) [Relay][Op] Add instance norm op (apache#4004) [QNN][Relay] Calling Dialect passes from inside Relay Build API. (apache#3971) [RELAY/PASS] Fix the extent for the post_stmt in the loop partition (apache#3734) ...

vinx13 requested changes Sep 9, 2019

View reviewed changes

vinx13 approved these changes Sep 9, 2019

View reviewed changes

junrushao approved these changes Sep 10, 2019

View reviewed changes

SWu reviewed Sep 10, 2019

View reviewed changes

weberlo suggested changes Sep 10, 2019

View reviewed changes

MarisaKirisame force-pushed the crossentropy branch from d112e48 to 9f3c850 Compare September 19, 2019 02:51

vinx13 requested changes Sep 19, 2019

View reviewed changes

MarisaKirisame force-pushed the crossentropy branch from 9f3c850 to 3517d33 Compare October 1, 2019 18:45

MarisaKirisame force-pushed the crossentropy branch from 3517d33 to 9680232 Compare October 1, 2019 21:11

vinx13 self-assigned this Oct 3, 2019

MarisaKirisame added 2 commits October 2, 2019 21:52

save

d3adc78

save redo max test save address comment fix

address comment

e884dfc

MarisaKirisame force-pushed the crossentropy branch from 71986be to e884dfc Compare October 3, 2019 04:52

increase rtol

cec2500

address review comment

39bd02b

vinx13 approved these changes Oct 5, 2019

View reviewed changes

vinx13 merged commit 7d71dd8 into apache:master Oct 5, 2019

vinx13 added the status: accepted label Oct 5, 2019

MarisaKirisame deleted the crossentropy branch October 5, 2019 01:30

anijain2305 pushed a commit to anijain2305/tvm that referenced this pull request Oct 17, 2019

[Relay][Training] Add gradient for Crossentropy (apache#3925)

68b0eb0

* save save redo max test save address comment fix * address comment * increase rtol * address review comment

wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 18, 2019

[Relay][Training] Add gradient for Crossentropy (apache#3925)

7af55cf

* save save redo max test save address comment fix * address comment * increase rtol * address review comment

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

		reg.register_pattern("nn.bitserial_dense", reg.OpPattern.OUT_ELEMWISE_FUSABLE)

		pack_dtype, out_dtype, unipolar)


		def cross_entropy(predictions, targets):



		RELAY_REGISTER_OP("nn.cross_entropy")
		.describe(R"code(Computes cross entropy given preditions and targets.)code" TVM_ADD_FILELINE)

	<< "x shape=, " << x->shape
	<< "x shape = " << x->shape << ", "

		reg.register_pattern("nn.bitserial_dense", reg.OpPattern.OUT_ELEMWISE_FUSABLE)


		reg.register_schedule("nn.cross_entropy", schedule_injective)

		from tvm.relay.testing import check_grad


		def test_crossentropy_grad():

Conversation

MarisaKirisame commented Sep 9, 2019

Uh oh!

vinx13 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarisaKirisame commented Sep 9, 2019

Uh oh!

junrushao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarisaKirisame commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vinx13 commented Sep 15, 2019

Uh oh!

MarisaKirisame commented Sep 18, 2019

Uh oh!

vinx13 commented Sep 19, 2019

Uh oh!

vinx13 commented Sep 19, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarisaKirisame commented Sep 19, 2019

Uh oh!

vinx13 commented Sep 19, 2019

Uh oh!

vinx13 commented Sep 28, 2019

Uh oh!

MarisaKirisame commented Oct 1, 2019

Uh oh!

MarisaKirisame commented Oct 2, 2019

Uh oh!

vinx13 commented Oct 3, 2019

Uh oh!

MarisaKirisame commented Oct 3, 2019

Uh oh!

vinx13 commented Oct 3, 2019

Uh oh!

MarisaKirisame commented Oct 3, 2019

Uh oh!

vinx13 commented Oct 4, 2019

Uh oh!

MarisaKirisame commented Oct 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants