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

Implement FC layer with helper #4726

Merged
merged 16 commits into from
Oct 16, 2017

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Oct 11, 2017

No description provided.

return self.program.current_block().append_op(*args, **kwargs)

@property
def multiple_input(self):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

def multiple_input(self, input_name='input'):
   inputs = self.kwargs.get(input_name, [])

}
})

def bias_attr(self, size):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not all layers need bias_attr, so it seems unsuitable to put it here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If a layer does not need bias, in the layer function just do not invoke bias_attr.

return self.program.current_block().append_op(*args, **kwargs)

@property
def multiple_input(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

A layer may have more than one input, so this function shall take a str to indicate which input we want to get.

return inputs

@property
def input(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as multiple_input()

return inputs[0]

@property
def param_attr(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since there is more than one input, there is also supposed to be more than one parameter. And we need some method to distinguish them.

yield ipt, param_attr

@property
def input_dtype(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as multiple_input, we may have several inputs.

@reyoung reyoung force-pushed the feature/fc_layer_with_helper branch from 2c67fea to 03fc36c Compare October 15, 2017 21:46
input=hidden2, size=10, act='softmax', program=program)
cost = cross_entropy(input=predict, label=label, program=program)
avg_cost = mean(x=cost, program=program)
self.assertIsNotNone(avg_cost)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

# backward(avg_cost)

Copy link
Collaborator

@JiayiFeng JiayiFeng left a comment

Choose a reason for hiding this comment

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

I think we can merge this PR first, and keep updating the design if any drawback is found during the implementation of layers.

@reyoung reyoung merged commit 8e52b34 into PaddlePaddle:develop Oct 16, 2017
type='elementwise_sub',
inputs={'X': [input],
'Y': [label]},
outputs={'Out': [minus_out]})
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to stop gradient for label

'Label': [label]},
outputs={'Y': [out]},
attrs=kwargs)
return out
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we automatically add python interface for operators? Unlike fc or square_error_cost, the python part of cross_entropy does not provide extra function.

__all__ = ['fc_layer', 'data_layer', 'cross_entropy']


def fc_layer(input,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we follow the current name convention in v2 api, which is fc() for fc layer

from paddle.v2.framework.framework import OpProtoHolder, Variable
import re

__all__ = ['fc_layer', 'data_layer', 'cross_entropy']
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should separate different layers into different files.

@reyoung reyoung deleted the feature/fc_layer_with_helper branch October 28, 2017 22:15
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

3 participants