Rename real to scalar_t. by ezyang · Pull Request #11163 · pytorch/pytorch

ezyang · 2018-08-31T21:48:44Z

This is necessary to allow us to use the complex header
which defines real (and is very sad if real is macro'ed).

We should also fix accreal, ureal, Real and REAL, but
only 'real' is the real blocker.

codemod -d aten/src/TH --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t
codemod -d aten/src/THC --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t
codemod -d aten/src/THNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t
codemod -d aten/src/THCUNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t

Signed-off-by: Edward Z. Yang ezyang@fb.com

facebook-github-bot

ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

This is necessary to allow us to use the complex header which defines real (and is very sad if real is macro'ed). We should also fix accreal, ureal, Real and REAL, but only 'real' is the real blocker. ``` codemod -d aten/src/TH --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THC --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THCUNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d torch/csrc/generic --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t ``` Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ssnl

Changes mostly looking good, except for a couple places where codemod shouldn't change things (e.g., in some comments). I commented at places I noticed, but I'm not sure if I didn't miss something.

A ~~real~~ scalar_t concern I have is THCNumerics.cuh where we define

template <typename scalar_t>
static inline __host__ __device__ scalar_t powi(scalar_t a, scalar_t b);

gcc7 complains when I do

<source>:7:23: error: expected nested-name-specifier before 'double'

     template<typename double>

                       ^~~~~~

It seems that it doesn't cause problem currently. But I wonder if this will be problematic if in future someone include this header in some file.

aten/src/TH/generic/THTensorApply.hpp

aten/src/TH/generic/THTensorLapack.cpp

aten/src/THC/THCReduce.cuh

aten/src/THC/generic/THCTensorSort.cu

aten/src/THNN/generic/Col2Im.c

ssnl · 2018-09-01T18:36:36Z

Expanding all files gave me the most laggy chrome behavior I've ever experienced.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ezyang · 2018-09-02T02:49:33Z

Here's how I'm addressing the comments:

I grepped for ag "typename scalar_t" in the TH directories, and manually fixed all occurrences that popped up by replacing scalar_t with T
Fixed the referenced comment issues
Searched for "//.+scalar_t" to see if I missed any inline comment issues
Grepped the diff for /*, which actually did bring up some more comment issues

Thinking about the template parameter problem, I'm actually not sure how wise it is to rename this as scalar_t, because that's our canonical name for our templated methods, which means that if we include any of the template<typename scalar_t> headers from inside generic TH, we'll twing the error. I guess this shouldn't actually happen.

facebook-github-bot

ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

facebook-github-bot

ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

ezyang · 2018-09-02T04:41:08Z

Needs fbcode changes.

ssnl · 2018-09-02T05:37:28Z

New commits looks good! Let me know if you have the fbcode changes ready and I'll review.

Summary: This is necessary to allow us to use the complex header which defines real (and is very sad if real is macro'ed). We should also fix accreal, ureal, Real and REAL, but only 'real' is the real blocker. ``` codemod -d aten/src/TH --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THC --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THCUNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t ``` Signed-off-by: Edward Z. Yang <ezyang@fb.com> Pull Request resolved: pytorch/pytorch#11163 Reviewed By: SsnL Differential Revision: D9619906 Pulled By: ezyang fbshipit-source-id: 922cb3a763c0bffecbd81200c1cefc6b8ea70942

Summary: This is necessary to allow us to use the complex header which defines real (and is very sad if real is macro'ed). We should also fix accreal, ureal, Real and REAL, but only 'real' is the real blocker. ``` codemod -d aten/src/TH --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THC --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t codemod -d aten/src/THCUNN --extensions c,cc,cpp,cu,cuh,h,TARGETS,py,hpp '\breal\b' scalar_t ``` Signed-off-by: Edward Z. Yang <ezyang@fb.com> Pull Request resolved: pytorch#11163 Reviewed By: SsnL Differential Revision: D9619906 Pulled By: ezyang fbshipit-source-id: 922cb3a763c0bffecbd81200c1cefc6b8ea70942

ezyang requested review from apaszke, colesbury, gchanan, soumith and zdevito as code owners August 31, 2018 21:48

ezyang force-pushed the pr/real-to-scalar-t branch 2 times, most recently from 7e73a7e to efc10f9 Compare August 31, 2018 22:02

facebook-github-bot reviewed Aug 31, 2018

View reviewed changes

ezyang added 2 commits September 1, 2018 09:18

Fix HIPify as well.

be90c56

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ssnl reviewed Sep 1, 2018

View reviewed changes

ezyang added 3 commits September 1, 2018 19:43

Remove scalar_t from template parameters; make them T instead.

08a535f

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Fix incorrect translations of comments

62602c5

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Fix some multiline comment issues

a8d17be

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ezyang force-pushed the pr/real-to-scalar-t branch from bc11ca4 to a8d17be Compare September 2, 2018 02:50

facebook-github-bot reviewed Sep 2, 2018

View reviewed changes

Fix a missing T transform

9e32e39

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

facebook-github-bot reviewed Sep 2, 2018

View reviewed changes

facebook-github-bot closed this in 0a8c8c1 Sep 2, 2018

PhilippPelz mentioned this pull request Oct 17, 2018

Integrating complex tensors #755

Closed

10 tasks

ezyang added the merged label Jun 26, 2019

Conversation

ezyang commented Aug 31, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

ssnl 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!

ssnl commented Sep 1, 2018

Uh oh!

ezyang commented Sep 2, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

ezyang commented Sep 2, 2018

Uh oh!

ssnl commented Sep 2, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants