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

Added support for import/export of Python layer #78

Merged
merged 5 commits into from
Jun 21, 2017
Merged

Added support for import/export of Python layer #78

merged 5 commits into from
Jun 21, 2017

Conversation

utsavgarg
Copy link
Contributor

Allows to import and export any custom Python layer

Copy link
Contributor

@virajprabhu virajprabhu left a comment

Choose a reason for hiding this comment

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

Also add test cases for this layer (and all new layers that we add hereon)

if (layer.loss_weight):
params['loss_weight'] = layer.loss_weight[0]
if (not layer.bottom):
params['endPoint'] = '1, 0'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not very clear. Please add a comment explaining this convention.

if (layer.python_param.layer):
params['layer'] = layer.python_param.layer
if (layer.python_param.param_str):
params.update(eval(layer.python_param.param_str))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the update? (just curious)

Copy link
Contributor Author

@utsavgarg utsavgarg Jun 20, 2017

Choose a reason for hiding this comment

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

update adds all keys from that dictionary as keys to the params dictionary.

@@ -274,6 +274,11 @@ button.close {
margin-right:-10px;
}

#Python {
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to name this something more meaningful?

@@ -69,6 +69,12 @@ let i = null, layerId = null, parentId = null, inputLength = null, outputLength
const dataLayers = ['ImageData', 'Data', 'HDF5Data', 'Input', 'WindowData', 'MemoryData', 'DummyData'];
// finding the input layers to start DFS
Object.keys(net).forEach(layerId => {
if (net[layerId].info.type == 'Python'){
if (net[layerId].params.endPoint == "1, 0"){
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment what "1, 0" is? In general please explain such hardcoded conventions whenever used && not obvious.

hasParamStr = True
if 'param_str' not in python_param.keys():
python_param['param_str'] = {}
if isinstance(layerParams[param], str) and ',' in layerParams[param]:
Copy link
Contributor

Choose a reason for hiding this comment

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

The second condition in this if is pretty hacky. Please fix. You might want to use a better convention throughout rather than the "1, 0", "0, 1" etc. if it doesn't break anything else.

params['endPoint'] = '1, 1'
for param in params:
if isinstance(params[param], list):
params[param] = str(params[param])[1:-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this slicing do?

Copy link
Contributor Author

@utsavgarg utsavgarg Jun 20, 2017

Choose a reason for hiding this comment

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

After converting the array to str, [1:-1] will remove the brackets.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f3aeef8 on utsavgarg:python into ** on Cloud-CV:master**.

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