Adding support for the sigmoid_cross_entropy_with_logits operator#4448
Conversation
abhinavarora
requested review from
jacquesqiao,
qingqing01,
reyoung and
zchen0211
| namespace paddle { | ||
| namespace operators { | ||
|
|
||
| using Tensor = framework::Tensor; |
There was a problem hiding this comment.
use using in the header file violate the google style, see the detail in Namespace
Do not use Namespace aliases at namespace scope in header files except in explicitly marked internal-only namespaces, because anything imported into a namespace in a header file becomes part of the public API exported by that file.
By the way, I think
using framework::Tensor;is enough here.
There was a problem hiding this comment.
Thank you for pointing this out. I will remove it.
| "(Tensor, default Tensor<float>), a 2-D tensor of the same type " | ||
| "and shape as X. This input is a tensor of probabalistic labels " | ||
| "for each logit"); | ||
| AddOutput("Y", |
There was a problem hiding this comment.
single output name prefers to Out, to be unified with other operators.
There was a problem hiding this comment.
Thank you for the suggestion. Fixed in ab15457
| sigmoid_X = expit(self.inputs['X']) | ||
| term1 = self.inputs['Labels'] * np.log(sigmoid_X) | ||
| term2 = (1 - self.inputs['Labels']) * np.log(1 - sigmoid_X) | ||
| self.outputs = {'Y': -term1 - term2} |
There was a problem hiding this comment.
-term1 - term2to
- term1 - term2add a space
There was a problem hiding this comment.
Thank you for the comment. Fixed in ab15457 .
| } | ||
| }; | ||
|
|
||
| // TODO(aroraabhinav) : Complete proto documentation |
There was a problem hiding this comment.
Thank you for pointing this out. I will remove this.
dzhwinter
left a comment
dzhwinter
left a comment
There was a problem hiding this comment.
LGTM!
by the way, we'd better implement the new operator through the fundamental math operators. That's will save a lot of test code and easier for optimize. :)
4 participants
This PR closes #4318