Expose custom attributes from C++ functions by lantiga · Pull Request #1405 · pytorch/pytorch

lantiga · 2017-04-29T15:42:24Z

This PR demonstrates a possible way to expose custom attributes from C++ functions.
Motivation: right now functions implemented in C++ do not expose function-specific attributes or methods (e.g. stride for ConvForward). This makes it impossible to get such attributes while traversing a computation graph.

Currently only convolution attributes have been exposed, with the aim of illustrating the approach. If the proposal gets a thumbs up, I'll proceed and expose attributes for the rest of the C++ functions.

As agreed with @apaszke on Slack #autograd-internals, this PR points to the autograd branch.

albanD

I am not sure how big this functions folder is going to grow. We may want to separate the attributes code in a separate sub-folder to keep this one with just the actual function implementations?

torch/csrc/autograd/python_cpp_function.cpp

  type.tp_call = THPCppFunction_call;
-  type.tp_methods = THPCppFunction_methods;
-  type.tp_getset = THPCppFunction_properties;
+  type.tp_methods = function_methods ? function_methods : attributes::default_methods;


torch/csrc/autograd/functions/convolution_attr.h

+namespace attributes {
+
+template<class T>
+PyObject* conv_stride(THPCppFunction* self, PyObject* hook)


torch/csrc/autograd/python_variable.cpp

 {
  auto& var = *self->cdata;
-  return PyInt_FromLong(var.output_nr);
+  return PyLong_FromLong(var.output_nr);


torch/csrc/autograd/functions/convolution_attr.h

+namespace attributes {
+
+template<class T>
+PyObject* conv_stride(THPCppFunction* self, PyObject* hook)


torch/csrc/autograd/python_cpp_function_attr.cpp

+
+namespace attributes {
+
+PyObject* next_functions(THPCppFunction* self, PyObject* hook)


…uild" This reverts commit a9a35ff.

…uild" This reverts commit cef68e8.

lantiga · 2017-04-30T18:40:14Z

Ok. I have implemented a getTupleAttr and getValueAttr, to avoid code repetition for exposing attributes. I've also removed _attr.* files, as a result of the code shrink. For the same reason I got rid of the attributes namespace.

The one thing I still have to do is to inherit default attributes.

lantiga · 2017-04-30T21:03:41Z

I played with a few C++ possibilities for concatenating default_properties to the ones passed to _initFunctionPyTypeObject, but the macro solution felt cleaner. I'll be happy to see C++ alternatives though.

At this point, all changes that were suggested are in.

lantiga · 2017-05-02T05:45:01Z

New autograd merged, great!
Should I proceed rebasing against master and send a new PR? /cc @soumith @apaszke

soumith · 2017-05-02T11:51:34Z

yes please. rebase against master, send a PR there.

@crcrpar

* Gradient clipping routine with fused kernels Identical API as PyTorch. Falls back to PyTorch impl when not computing L2 norm. * Add unit test for gradient clipping * Add fp16 case to gradient clipping unit test * Tweaks to grad clipping unit test Review suggestions from @crcrpar * Debug gradient clipping tests When checking that incorrect results produce assertion errors, make sure to generate a discrepancy outside the range of numerical error.

lantiga added 4 commits April 29, 2017 17:40

Added custom properties and methods to functions (conv for now).

5449849

Add missing include to fix macOS build

e4be3e3

Replace PyInt_FromLong with PyLong_FromLong to fix Python 3 build

cef68e8

Replace PyInt_FromLong with PyLong_FromLong to fix Python 3 build

a9a35ff

albanD reviewed Apr 29, 2017

View reviewed changes

apaszke reviewed Apr 29, 2017

View reviewed changes

lantiga added 4 commits April 30, 2017 17:11

Revert "Replace PyInt_FromLong with PyLong_FromLong to fix Python 3 b…

a8857be

…uild" This reverts commit a9a35ff.

Revert "Replace PyInt_FromLong with PyLong_FromLong to fix Python 3 b…

f345953

…uild" This reverts commit cef68e8.

Include THP for PyInt macros

0eeefd4

Add helpers to avoid code repetition for attributes. Remove attr files.

932ac3a

lantiga added 3 commits April 30, 2017 21:04

Include THP for PyInt_ functions in Python 3

fe8b152

Fix formatting and typo

623df11

Introduce macros to avoid including default props and methods by hand

7b64a8a

soumith closed this May 2, 2017

lantiga mentioned this pull request May 2, 2017

Expose custom attributes from C++ functions #1430

Merged

ezyang added the open source label Jun 24, 2019


		namespace attributes {

		PyObject* next_functions(THPCppFunction* self, PyObject* hook)

Conversation

lantiga commented Apr 29, 2017

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

lantiga commented Apr 30, 2017

Uh oh!

lantiga commented Apr 30, 2017

Uh oh!

lantiga commented May 2, 2017

Uh oh!

soumith commented May 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants