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

Made templates a little smarter for those functions which require a s… #499

Merged
merged 1 commit into from
Apr 12, 2016

Conversation

translunar
Copy link
Member

The return dtype for functions that return a magnitude doesn't need to be specified any longer.

@translunar
Copy link
Member Author

@wlevine Any chance you can take a look at this? Running into some problems with the extensions, since I don't have a working installation of ATLAS on my machine.

@@ -149,9 +149,9 @@ inline void cblas_asum(const int N, const void* X, const int incX, void* sum) {
* complex64 -> float or double
* complex128 -> double
*/
template <typename ReturnDType, typename DType>
template <typename DType, typename MDType = typename MagnitudeDType<DType>::type>
inline ReturnDType nrm2(const int N, const DType* X, const int incX) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis should be MDType

@wlevine
Copy link

wlevine commented Apr 9, 2016

Also, the template tables for cblas_nrm2 need updating in math_lapacke.cpp, math_atlas.cpp (and also math.cpp though somehow this one is compiling already?)

…eparate return DType by adding MagnitudeDType. Also added a magnitude function to replace std::abs and abs -- but mostly it just calls those.
@translunar
Copy link
Member Author

@v0dro Mind merging? Trying to follow the same rules I've asked other maintainers to follow. =)

Looks like the failure is just a Travis problem.

@v0dro v0dro merged commit 1192117 into SciRuby:master Apr 12, 2016
@v0dro
Copy link
Member

v0dro commented Apr 12, 2016

works. looks good :)

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

Successfully merging this pull request may close these issues.

3 participants