Skip to content

Conversation

@smessmer
Copy link
Contributor

@smessmer smessmer commented Apr 15, 2019

Stack:
    :white_circle:  #19280 Split function schema parser from operator  💛
    :white_circle:  #19281 Fixing function schema parser for Android  💛
    :white_circle:  #19282 Move function schema parser to ATen/core build target  💚
    :white_circle:  #19283 String-based schemas in op registration API  💛
    :white_circle:  #19284 Allow ops without tensor args if only fallback kernel exists  💛
    :white_circle:  #19285 Add either type  💛
    :black_circle:  #19286 Allow registering ops without specifying the full schema  💛
    :white_circle:  #19287 Use string based schema for exposing caffe2 ops  💛
    :white_circle:  #19288 Add some tests  💛
    :white_circle:  #19289 Optional inputs and outputs  💛
    :white_circle:  #19290 Add tests for argument types  💛

The operator registration API now allows registering an operator by only giving the operator name and not the full operator schema,
as long as the operator schema can be inferred from the kernel function.

Differential Revision: D14931921

Differential Revision: D14931921
Differential Version: 79493183
Differential Revision: D14931921
Differential Version: 79547652
Differential Revision: D14931921
Differential Version: 79570169
Copy link
Collaborator

@dzhulgakov dzhulgakov left a comment

Choose a reason for hiding this comment

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

Looks pretty idiomatic :)

Given that you just check whether '(' is in the schema - couldn't we just use str.find() first and then call one method or another? That way we could have saved ourselves need for Either.

But since you've already added it, I don't mind

Differential Revision: D14931921
Differential Version: 79580377
@smessmer
Copy link
Contributor Author

We'd still need the either type because - even if using str.find(), I'd like that logic to live inside the parser header and be returned by parseSchemaOrName(). There will likely be other places where users can specify either a full schema or just a name, and I don't want to repeat the str.find() logic there.

Differential Revision: D14931921
Differential Version: 79637869
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 5ca22cc.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Apr 18, 2019
Summary:
Pull Request resolved: pytorch/pytorch#19286

The operator registration API now allows registering an operator by only giving the operator name and not the full operator schema,
as long as the operator schema can be inferred from the kernel function.

Reviewed By: dzhulgakov

Differential Revision: D14931921

fbshipit-source-id: 3776ce43d4ce67bb5a3ea3d07c37de96eebe08ba
zhangguanheng66 pushed a commit to zhangguanheng66/pytorch that referenced this pull request May 6, 2019
Summary:
Pull Request resolved: pytorch#19286

The operator registration API now allows registering an operator by only giving the operator name and not the full operator schema,
as long as the operator schema can be inferred from the kernel function.

Reviewed By: dzhulgakov

Differential Revision: D14931921

fbshipit-source-id: 3776ce43d4ce67bb5a3ea3d07c37de96eebe08ba
@ezyang ezyang deleted the export-D14931921 branch May 30, 2019 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants