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

Support SperableConv2D and one hot crossentropy #1944

Merged
merged 16 commits into from Dec 12, 2017

Conversation

yiheng
Copy link
Contributor

@yiheng yiheng commented Nov 27, 2017

What changes were proposed in this pull request?

  1. Add a new layer type SpatialSeperableConvolution
  2. Add a new criterion type cross entropy which accepts onehot tensor as target input
  3. Support to load tf depthwise operations

How was this patch tested?

unit test

@yiheng yiheng force-pushed the keras_layer branch 5 times, most recently from 62cc9cb to 3234520 Compare December 6, 2017 07:26
@yiheng
Copy link
Contributor Author

yiheng commented Dec 6, 2017

PR validation pass

"""
|input_tensor = Input(shape=[3])
|target_tensor = Input(shape=[3])
|loss = categorical_crossentropy(target_tensor, input_tensor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as some other guys put "input_tensor" at the first position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the sequence in keras API

@yiheng yiheng requested a review from i8run December 11, 2017 01:56
p1.almostEqual(p2, 1e-3) should be(true)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a serialization test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

Seq(Tensor[Float](4, 24, 24, 3).rand(), filter),
0
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a serialization test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need for tf loaders, but have added for the new added operation

Seq(input, filterSize, gradOutput),
0, 1e-3
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a serialization test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need for tf loaders, but have added for the new added operation

Seq(size, filter, Tensor[Float](4, 23, 23, 6).rand()),
0
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a serialization test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need for tf loaders, but have added for the new added operation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we save model, will tf loaders not be saved ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. It is only used when load tf model, which will generate an operation. I have added for the operations test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yiheng it doesn't look so when I ran the SessionSpec unit test, seems loaders are part of the graph meaning it will be serialized as well

@zhichao-li
Copy link
Contributor

zhichao-li commented Dec 12, 2017

Any progress for this? one hot crossentropy looks great. Looking forward to using it.

@yiheng
Copy link
Contributor Author

yiheng commented Dec 12, 2017

@zhichao-li as required by the reviewer, need to add serialization test

@yiheng yiheng merged commit 36cd62b into intel-analytics:master Dec 12, 2017
@yiheng yiheng deleted the keras_layer branch December 15, 2017 03:32
Le-Zheng pushed a commit to Le-Zheng/BigDL that referenced this pull request Oct 20, 2021
* Revert "Create deploy_googlenet_places365.prototxt"

This reverts commit ceaa3f9346faa66e3c4355590d91c4eff5d78e2c.

* fix piip python test issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants