Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

add contrib package #5499

Merged
merged 2 commits into from Mar 21, 2017
Merged

add contrib package #5499

merged 2 commits into from Mar 21, 2017

Conversation

piiswrong
Copy link
Contributor

@piiswrong piiswrong commented Mar 20, 2017

@precedenceguo @zhreshold
Moved extra operators to contrib. Had to fix lint, please check if I made any error.
BTW do you have unittests for these ops?

@mli
Copy link
Member

mli commented Mar 20, 2017

if i understand correctly, we put ops in contrib if they are new and not well tested. once they are mature, we will consider to move them into core.

then do we really want to have contrib in mx.sym.contrib.xxx? if we move the ops,
then users need to update their codes.

@piiswrong
Copy link
Contributor Author

contrib is for things that we don't give backward compatibility promise. When ops move to core interface/behavior may change

@zhreshold
Copy link
Member

Does it mean all contrib ops are enabled by default?
I agree that some ops that are specific to certain tasks should be put somewhere else, but do you have a rule that what should be in and what should not? @piiswrong
BTW, I don't think backward compatibility is a big issue for those Ops metioned.

@piiswrong
Copy link
Contributor Author

It's basically a namespace that indicates limited guarantees. volatile interface/behavior, untested, etc

@ijkguo
Copy link
Contributor

ijkguo commented Mar 21, 2017

I think it is better not to include them by default. Perhaps we can have a ENABLE_CONTRIB Makefile config to hide these operators since It could be confusing without examples.

Operators not core to deep learning are moved to this contrib package so that newcomers do not need to know them. For example, Correlation, BilinearSampler, GridGenerator, SpatialTransformer. If some operators get used frequently like BatchNorm, they could be moved to the main package.

@mli As to ROIPooling, smooth_l1, Proposal, MultiboxPrior, etc, I think these are special operators only for one purpose. Their usage is limited and one-time. It would not affect many users.

@piiswrong
Copy link
Contributor Author

We can probably add an option to disable it in the future when compile time is too long. For now keeping them in contrib is fine.

@piiswrong piiswrong merged commit 289ff56 into apache:master Mar 21, 2017
@fullfanta
Copy link
Contributor

@piiswrong

please fix the bug on faster-rcnn example.

At setup.py in python, packages contain 'mxnet.contrib'.

But at faster-rcnn example(https://github.com/dmlc/mxnet/blob/master/example/rcnn/rcnn/symbol/symbol_vgg.py), it uses mx.symbol.contrib.Proposal rather than mx.contrib.symbol.Proposal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants