Skip to content

Switch C10_EXPORT_CAFFE2_OP_TO_C10 to new operator registration API #43409

Open
@ezyang

Description

@ezyang

caffe2/core/export_caffe2_op_to_c10.h is still using the old API. Among other problems, @jackm321 ran into trouble because these were all having aliasing inferred conservatively when actually most of these functions are pure. Let's switch it to the new API.

1Change the macros to use the new operator API (c.f. https://pytorch.org/tutorials/advanced/dispatcher.html). For the most part, you should be able to do this directly in the macro definitions themselves, but for the C10_EXPORT_CAFFE2_OP_TO_C10_SCHEMA_ONLY call, you will have to use TORCH_LIBRARY_FRAGMENT_THIS_API_IS_FOR_PER_OP_REGISTRATION_ONLY to relax the restriction that all definitions are made in one place.

One fly in the ointment is the use of TORCH_LIBRARY_FRAGMENT_THIS_API_IS_FOR_PER_OP_REGISTRATION_ONLY. When we designed TORCH_LIBRARY, we added the constraint that all operators for a namespace had to be declared in one place. However, the Caffe2 operators are pretty decentralized, and live in both fb/ and non-fb folders. This is making me wonder if we should keep caffe2 as a special namespace which is allowed to be registered anywhere you want (in keeping with Caffe2's historical nature). Otherwise, we will have to go on a renamespacing spree for all fb/ folder caffe2 operators, which seems like a lot of work for not much gain.

Activity

added
module: bootcampWe plan to do a full writeup on the issue, and then get someone to do it for onboarding
on Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    caffe2module: bootcampWe plan to do a full writeup on the issue, and then get someone to do it for onboarding

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Switch C10_EXPORT_CAFFE2_OP_TO_C10 to new operator registration API · Issue #43409 · pytorch/pytorch