Commit Graph

3 Commits

Author SHA1 Message Date
Scott Wolchok
4a0d17ba2d [PyTorch][codemod] Replace immediately-dereferenced expect calls w/expectRef (#50228)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50228

`fastmod -m 'expect(<((at|c10)::)?\w+Type>\(\)\s*)->'
'expectRef${1}.'`
Presuming it builds, this is a safe change: the result of `expect()`
wasn't being saved anywhere, so we didn't need it, so we can take a
reference instead of a new `shared_ptr`.
ghstack-source-id: 119782961

Test Plan: CI

Reviewed By: SplitInfinity

Differential Revision: D25837374

fbshipit-source-id: 86757b70b1520e3dbaa141001e7976400cdd3b08
2021-01-13 16:13:55 -08:00
Meghan Lele
4d3c12d37c [JIT] Print better error when class attribute IValue conversion fails (#50255)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50255

**Summary**
TorchScript classes are copied attribute-by-attribute from a py::object into
a `jit::Object` in `toIValue`, which is called when copying objects from
Python into TorchScript. However, if an attribute of the class cannot be
converted, the error thrown is a standard pybind error that is hard to
act on.

This commit adds code to `toIValue` to convert each attribute to an
`IValue` inside a try-catch block, throwing a `cast_error` containing
the name of the attribute and the target type if the conversion fails.

**Test Plan**
This commit adds a unit test to `test_class_type.py`
based on the code in the issue that commit fixes.

**Fixes**
This commit fixes #46341.

Test Plan: Imported from OSS

Reviewed By: pbelevich, tugsbayasgalan

Differential Revision: D25854183

Pulled By: SplitInfinity

fbshipit-source-id: 69d6e49cce9144af4236b8639d8010a20b7030c0
2021-01-11 14:04:26 -08:00
Luca Wehrstedt
1ac05cfe01 Remove DataPtr extractor from CUDAFuture (#48840)
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/48840

The CUDAFuture class needs to inspect the values it contains in order to extract its tensors (in fact, the DataPtrs backing those). These are needed first to determine what CUDA devices back those tensors, so that an event for each such device can be recorded; and later to record these DataPtrs with the CUDA caching allocator if they are used in other streams.

This became complicated when Python was added to the mix, because to inspect a Python object we need to acquire the GIL, but we couldn't do so from code that was supposed to also work in C++-only mode. The solution was for users to provide a custom way to extract DataPtrs, so that the PythonFutureWrapper could install such a custom Python-aware one. This was the DataPtr extractor.

In https://github.com/pytorch/pytorch/pull/48502 a different suggestion was proposed. At its root, it consists in adding support for IValues of type PyObject to the visit() and getSubValues() methods. In order to deal with the GIL, we do this through a virtual method: PyObjectHolder, which is the base class, is available also in C++-only mode, and thus defines this method but leaves it unimplemented; ConcretePyObjectHolder, which is the subclass, is only included in Python mode, and thus it can implement that method, acquire the GIL, and do what it's supposed to.

In my opinion, this approach is just brilliant! Thank wanchaol for proposing it! It hides the complexity of dealing with Python inside getSubValues(), where it can be done properly, thus simplifying enormously the CUDAFuture and the PythonFutureWrapper classes.
ghstack-source-id: 118704935

Test Plan: Unit tests

Reviewed By: wanchaol

Differential Revision: D25334355

fbshipit-source-id: 3f1d3bf6e6e8505a114c877fb9a6fcc3f68d91d3
2020-12-19 11:03:45 -08:00