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

[WIP] Various fixes, mostly SameDiff/Nd4j #110

Merged
merged 32 commits into from Aug 21, 2019

Conversation

@rnett
Copy link
Collaborator

commented Aug 12, 2019

Fixes eclipse#8031.
Should also fix eclipse#7676 (although some of the javadocs are rather brief).

Going to add add a few more before merging.

rnett added 3 commits Aug 12, 2019
Nd4j pad update
Signed-off-by: Ryan Nett <rnett@skymind.io>
switched from guava Immutables to Collections.unmodifiableList/Map
Signed-off-by: Ryan Nett <rnett@skymind.io>
javadoc
Signed-off-by: Ryan Nett <rnett@skymind.io>

@rnett rnett self-assigned this Aug 12, 2019

rnett added 10 commits Aug 13, 2019
use new pad
Signed-off-by: Ryan Nett <rnett@skymind.io>
conv tests use OpValidation
Signed-off-by: Ryan Nett <rnett@skymind.io>
deconv3d overrides
Signed-off-by: Ryan Nett <rnett@skymind.io>
test fix for the new pad method
Signed-off-by: Ryan Nett <rnett@skymind.io>
more test fixes
Signed-off-by: Ryan Nett <rnett@skymind.io>
more test fixes
Signed-off-by: Ryan Nett <rnett@skymind.io>
rename SameDiff function methods to op (except for the actual SameDif…
…f function ones)

Signed-off-by: Ryan Nett <rnett@skymind.io>
more pad overloads, test fix
Signed-off-by: Ryan Nett <rnett@skymind.io>
test updates
Signed-off-by: Ryan Nett <rnett@skymind.io>
conv1d test
Signed-off-by: Ryan Nett <rnett@skymind.io>
@rnett

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 15, 2019

The added conv1d test fails because of eclipse#8085, the actual equality check passes. It should pass as soon as that is fixed.

@rnett

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2019

Also removed Conv1D TF import, as its not used (Tensorflow has no Conv1D op, uses transform and Conv2D).

rnett added 2 commits Aug 16, 2019
remove Conv1D tf import (there isn't a TF conv1d op)
Signed-off-by: Ryan Nett <rnett@skymind.io>
remove numThreads from Nd4j
Signed-off-by: Ryan Nett <rnett@skymind.io>
@rnett

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2019

For eclipse#8100, I didn't touch ImportClassMapping or LegacyOpMapper, but everything else has been updated.

rnett added 2 commits Aug 16, 2019
replace Old ops with their newer versions, deprecate ones that haven'…
…t already been deprecated

Signed-off-by: Ryan Nett <rnett@skymind.io>
remove use of setNumThreads
Signed-off-by: Ryan Nett <rnett@skymind.io>
@rnett

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2019

Fixes eclipse#8102.
Fixes eclipse#8100.

rnett added 3 commits Aug 17, 2019
fix for Reverse and ATan2
Signed-off-by: Ryan Nett <rnett@skymind.io>
fix test for wrong equals type
Signed-off-by: Ryan Nett <rnett@skymind.io>
well it works now
Signed-off-by: Ryan Nett <rnett@skymind.io>

@rnett rnett marked this pull request as ready for review Aug 17, 2019

@rnett rnett requested a review from AlexDBlack Aug 17, 2019

@AlexDBlack
Copy link

left a comment

A couple of minor changes required, but overall LGTM 👍

Let's also remove the Old* ops (arithmetic/comparison - add, min etc, and boolean - equalTo etc; also OldIdentity/Reverse/Convolution) entirely, I don't think there's a need for them at the java level anymore at this point
Normally we'd deprecate for a release or two, but Given they're already called "Old*" it shouldn't hopefully be too hard for users to migrate their code...

@AlexDBlack

This comment has been minimized.

Copy link

commented Aug 19, 2019

LayerOpValidaiton.testConv1dBasic is also failing here but passing on master

rnett added 6 commits Aug 19, 2019
better javadocs
Signed-off-by: Ryan Nett <rnett@skymind.io>
NonNulls
Signed-off-by: Ryan Nett <rnett@skymind.io>
better array literal
Signed-off-by: Ryan Nett <rnett@skymind.io>
re-add tf import stuff (will remove later)
Signed-off-by: Ryan Nett <rnett@skymind.io>
conv1d config load fix
Signed-off-by: Ryan Nett <rnett@skymind.io>
rnett added 5 commits Aug 20, 2019
partial config usage changes
Signed-off-by: Ryan Nett <rnett@skymind.io>
remove Old op classes
Signed-off-by: Ryan Nett <rnett@skymind.io>
config property fixes
Signed-off-by: Ryan Nett <rnett@skymind.io>
removed one too many ops
Signed-off-by: Ryan Nett <rnett@skymind.io>

@rnett rnett requested a review from AlexDBlack Aug 21, 2019

@AlexDBlack
Copy link

left a comment

LGTM - as long as tests are passing and full build is OK, this should be good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.