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

enable setting return type for operators in registry #1390

Merged
merged 12 commits into from
Feb 2, 2016
Merged

Conversation

hjk41
Copy link
Contributor

@hjk41 hjk41 commented Feb 1, 2016

Most operators returns one Symbol, but some like slice_channel returns array of Symbols. This patch enables setting return type to "Symbol" or "Symbol[]" in the registry, so that we can get the return type in MXSymbolGetAtomicSymbolInfo()

@piiswrong
Copy link
Contributor

Can this be accomplished by exposing num visible outputs?

@hjk41
Copy link
Contributor Author

hjk41 commented Feb 1, 2016

@piiswrong nope. The number of outputs for slice_channel is not fixed. It depends on the parameters.

@piiswrong
Copy link
Contributor

Is the return type of Slice still an array if there is only one output?
BTW, why do you need the return type before creating a symbol?

@hjk41
Copy link
Contributor Author

hjk41 commented Feb 1, 2016

virtual void Forward(const OpContext &ctx,
                       const std::vector<TBlob> &in_data,
                       const std::vector<OpReqType> &req,
                       const std::vector<TBlob> &out_data,
                       const std::vector<TBlob> &aux_args)

The output of our operators are always array of tblobs. But for most of the operators, we just have one element in the output.
This patch does not affect any existing code, it just enables us to set the information when we register the op properties. And the op properties are just static information of the op. I am currently relying on this information to generate the CPP wrappers for the operators. I think it is also used to generated python doc. Fixing this will allow me to get the return type of the SliceChannel operator right, and it should also fix the python doc.

@piiswrong
Copy link
Contributor

I see.

hjk41 added a commit that referenced this pull request Feb 2, 2016
enable setting return type for operators in registry
@hjk41 hjk41 merged commit eeecc52 into apache:master Feb 2, 2016
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.

2 participants