Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THTensor_(*) for abs, sqrt and tanh #403

Merged
merged 1 commit into from
Sep 26, 2015
Merged

THTensor_(*) for abs, sqrt and tanh #403

merged 1 commit into from
Sep 26, 2015

Conversation

szagoruyko
Copy link
Member

removing legacy code

soumith added a commit that referenced this pull request Sep 26, 2015
THTensor_(*) for abs, sqrt and tanh
@soumith soumith merged commit 6bfb7d1 into torch:master Sep 26, 2015
@soumith
Copy link
Member

soumith commented Sep 26, 2015

Thanks

@fmassa
Copy link
Contributor

fmassa commented Sep 27, 2015

There is actually a performance loss for Sqrt and Tanh with this PR.
Current pointwise torch functions are not paralellized, see https://github.com/torch/torch7/blob/master/lib/TH/generic/THTensorMath.c#L1854 and https://github.com/torch/torch7/blob/master/lib/TH/generic/THTensorMath.c#L1802
I agree though that nn functions should maximally rely on torch functions. In this case, shouldn't we add parallelization on pointwise functions in core torch ?
With current framework, I think it would require manually writing repeated code for each pointwise function, as it seems that one can not use preprocessor macros (#pragma omp parfor) inside a macro definition.

@hughperkins
Copy link
Contributor

With current framework, I think it would require manually writing repeated code for each pointwise function, as it seems that one can not use preprocessor macros (#pragma omp parfor) inside a macro definition.

Hmmm, plus one point in favor of Jinja2/Lua templates :-P (Mind you, this can also be argued to be true also for c++ templates too plausibly).

@fmassa
Copy link
Contributor

fmassa commented Sep 27, 2015

Well, there are workarounds to allow parallel functions inside a macro.
Here is first thing I tried that works (at least on gcc 4.6.3). fmassa/torch7@c0a6654
There are lots of compiler warnings (maybe because we are calling functions like tanh with float arguments, but it should be tanhf instead), but it seems to work.
Any better solutions that work on C99 ?

@hughperkins
Copy link
Contributor

seems like it is because you are sending a pointer to a function that takes a double, into a function that is expecting a pointer to a function that takes a float. Sent a pull request that seems to remove build warnings (no idea if it runs correctly or not though :-P ) fmassa/torch7#1

@szagoruyko
Copy link
Member Author

we have a fix for all math foo/foof cases, this PR was a preparation for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants