-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
7f1a929
to
d94ec27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please skip these section from your code review as it did not change
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/LstmBucketing.scala
Show resolved
Hide resolved
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/TrainCharRnn.scala
Show resolved
Hide resolved
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/TrainCharRnn.scala
Show resolved
Hide resolved
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/BucketIo.scala
Show resolved
Hide resolved
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/README.md
Outdated
Show resolved
Hide resolved
scala-package/examples/src/main/scala/org/apache/mxnetexamples/rnn/README.md
Outdated
Show resolved
Hide resolved
scala-package/examples/src/test/scala/org/apache/mxnetexamples/rnn/ExampleRNNSuite.scala
Outdated
Show resolved
Hide resolved
Add download: #11866 |
I would like this PR to wait until we have #11844 is merged in so we make sure that DataDesc works with RNN & Bucketing moduel |
So I think probably we cannot make this as |
@nswamy @lanking520 Thanks for the update :) |
c72dca0
to
9395600
Compare
9395600
to
4fbd52a
Compare
26c037a
to
9e420ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Unfortunately github makes looking at this far more confusing than it should be.
scala-package/examples/src/test/scala/org/apache/mxnetexamples/rnn/ExampleRNNSuite.scala
Show resolved
Hide resolved
@lanking520 thanks for fixing the RNN example, @andrewfayres Thanks for the review and yes indeed the whitespace/formatting changes make it hard to look at it. |
* initial fix for RNN * add CI test * add encoding format * scala style fix * update readme * test char RNN works * ignore the test due to memory leaks
* initial fix for RNN * add CI test * add encoding format * scala style fix * update readme * test char RNN works * ignore the test due to memory leaks
Description
This is the example written in new API in Scala. It include the following two tests:
RNN Example not supported for MXNet Scala #11571
@yzhliu @nswamy @andrewfayres
This example also include a change on the DataDesc by defining a default settings for shape size = 2 in
NT
formatChecklist
Essentials
Please feel free to remove inapplicable items for your PR.