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

AggregationSchemes for Maximum and Minimum. #1162

Merged
merged 4 commits into from
Nov 1, 2016

Conversation

dwf
Copy link
Contributor

@dwf dwf commented Oct 29, 2016

Surprised to find these didn't already exist. Fixes #129.

@dmitriy-serdyuk
Copy link
Contributor

It is issue #129 .

@dwf
Copy link
Contributor Author

dwf commented Oct 29, 2016

So it is.

@dwf
Copy link
Contributor Author

dwf commented Oct 29, 2016

I added a Concatenate scheme for completeness.

@dmitriy-serdyuk
Copy link
Contributor

dmitriy-serdyuk commented Nov 1, 2016

Sorry, forgot about this PR. It looks good. I have one question: do you think that concatenate aggregator should always create an extra dim? For example, in your test [4, 12, 7, 5] should be [[4, 12], [7, 5]].

@dwf
Copy link
Contributor Author

dwf commented Nov 1, 2016 via email

@dmitriy-serdyuk
Copy link
Contributor

Cool, LGTM.

@dmitriy-serdyuk dmitriy-serdyuk merged commit 1d6683f into mila-iqia:master Nov 1, 2016
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

2 participants