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

Remove calculate output shape from java side #151

Merged
merged 16 commits into from Aug 28, 2019

Conversation

@rnett
Copy link
Collaborator

commented Aug 22, 2019

Mostly fixes eclipse#7901.

A few ops have issues that prevent removing the method now (eclipse#8131, eclipse#8129, eclipse#8128).

"Fake" Java-only ops have a calculateOutputShape that throws a UnsupportedOperationException. Doing this in a method needed in each op isn't ideal, if there's an easy way to detect "fake" ops that would be easier, but I'm not aware of one.

rnett added 12 commits Aug 20, 2019
remove some unneeded java-side output shape calculations
Signed-off-by: Ryan Nett <rnett@skymind.io>
delete Broadcast
Signed-off-by: Ryan Nett <rnett@skymind.io>
delete Linear and Module,
Signed-off-by: Ryan Nett <rnett@skymind.io>
update Identity, HashCode, and NoOp
Signed-off-by: Ryan Nett <rnett@skymind.io>
removed Cast java-side shape function, added tests and SDVariable.isE…
…mpty

Signed-off-by: Ryan Nett <rnett@skymind.io>
ignoring test w/ issues on master
Signed-off-by: Ryan Nett <rnett@skymind.io>
noop needs more work, fixed BaseArithmeticBackprop and BaseDynamicTra…
…nsform ops

merge in master for c++ build fix

Signed-off-by: Ryan Nett <rnett@skymind.io>
fix EqualTo
Signed-off-by: Ryan Nett <rnett@skymind.io>
fix other cond ops
Signed-off-by: Ryan Nett <rnett@skymind.io>
"fake" ops calculateOutputShape() throws exception
Signed-off-by: Ryan Nett <rnett@skymind.io>
use c++ shape calc for Linspace
Signed-off-by: Ryan Nett <rnett@skymind.io>

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

@rnett rnett self-assigned this Aug 22, 2019

@rnett

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2019

Also note there is a test for cast on an empty array that NPEs. I think its due to not having offline shape inference rather than a cast issue, but its worth taking a look at.

@Test
@Ignore // casted shape is null
public void castShapeTestEmpty(){
SameDiff sd = SameDiff.create();
SDVariable x = sd.constant(Nd4j.empty(DataType.INT));
SDVariable casted = x.castTo(DataType.FLOAT);
assertEquals(casted.dataType(), DataType.FLOAT);
assertTrue(casted.isEmpty());
}

@AlexDBlack
Copy link

left a comment

Overall looks good, few small changes required.

rnett added 3 commits Aug 23, 2019
fix exception message, move most to BaseCompatOp
Signed-off-by: Ryan Nett <rnett@skymind.io>
remove SDVariable.isEmpty
Signed-off-by: Ryan Nett <rnett@skymind.io>
remove commented out code
Signed-off-by: Ryan Nett <rnett@skymind.io>

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

Merge branch 'master' into rn_remove_calculate_output_shape
# Conflicts:
#	nd4j/nd4j-backends/nd4j-api-parent/nd4j-api/src/main/java/org/nd4j/autodiff/samediff/SDVariable.java
#	nd4j/nd4j-backends/nd4j-api-parent/nd4j-api/src/main/java/org/nd4j/linalg/api/ops/impl/layers/Linear.java

@rnett rnett requested review from AlexDBlack and removed request for AlexDBlack Aug 27, 2019

@AlexDBlack
Copy link

left a comment

As long as tests pass - LGTM

@rnett rnett merged commit 2a14312 into master Aug 28, 2019

@rnett rnett deleted the rn_remove_calculate_output_shape branch Aug 28, 2019

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.