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

Add maxout op to tf.contrib.layers #5528

Closed
wants to merge 5 commits into from
Closed

Add maxout op to tf.contrib.layers #5528

wants to merge 5 commits into from

Conversation

gokceneraslan
Copy link
Contributor

No description provided.

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@mention-bot
Copy link

@gokceneraslan, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tensorflower-gardener, @zhangyaobit and @martinwicke to be potential reviewers.

@gokceneraslan
Copy link
Contributor Author

gokceneraslan commented Nov 10, 2016

Small demo:

image

@Mistobaan
Copy link
Contributor

add a reference to the paper? https://arxiv.org/abs/1302.4389

@teamdandelion teamdandelion added stat:awaiting tensorflower Status - Awaiting response from tensorflower awaiting review Pull request awaiting review labels Nov 29, 2016
@martinwicke
Copy link
Member

@ilblackdragon, can you review?

Copy link
Contributor

@ilblackdragon ilblackdragon left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Can you also please add tests. Specifically to check that reshape works correctly for 4-5d tensors with axis in the middle (as it's easy to mix up axis when reshaping).


Raises:
ValueError: if num_units is not multiple of number of features.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

the """ should be 2 spaces left.

if axis is None:
# Assume that channel is the last dimension
axis = -1
num_channels = shape[axis]
Copy link
Contributor

Choose a reason for hiding this comment

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

num_channels here can be None, e.g. undefined in shape inference. if it's None, you can still implement this via tf.shape() and tf.pack later.

scope: Optional scope for name_scope.

Returns:
A `Tensor` representing the results of the pooling operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

please describe shape of output tensor.

also may be in description add a formula e.g. out[..., i, ..., j] = max(in[...]) (similar how https://www.tensorflow.org/versions/r0.12/api_docs/python/nn.html#conv2d)

raise ValueError('number of features({}) is not '
'a multiple of num_units({})'
.format(num_channels, num_units))
shape[axis] = -1
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be = num_units?
[it's better not to have -1 in reshape, as it's easy to wrongly reshape and not know about it]

@ilblackdragon ilblackdragon added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower awaiting review Pull request awaiting review labels Dec 13, 2016
@drpngx
Copy link
Contributor

drpngx commented Jan 11, 2017

@gokceneraslan any luck with this?

@drpngx
Copy link
Contributor

drpngx commented Jan 15, 2017

Closing due to inactivity. Please re-open when updates are available.

@drpngx drpngx closed this Jan 15, 2017
@StefanoD
Copy link

@gokceneraslan Would be interested in this feature! Would be cool, if you would finish this PR! :)

@tiagofrepereira2012
Copy link
Contributor

Hi,

I'm interested in this feature.

Is it just a matter of implement some test units and improve the documentation?

@martinwicke
Copy link
Member

Yes, basically the work to do is to address @ilblackdragon's comments.

I saw you made a PR, but it was closed, not sure what happened.

@tiagofrepereira2012
Copy link
Contributor

I don't know what happened to; I'll reopen it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes stat:awaiting response Status - Awaiting response from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.