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

Fix cblas_nrm2 for complex types #438

Merged
merged 1 commit into from
Feb 1, 2016
Merged

Conversation

lokeshh
Copy link
Member

@lokeshh lokeshh commented Jan 24, 2016

Pass correct parameters to nrm2_complex_helper

Fixes #389

@v0dro
Copy link
Member

v0dro commented Jan 24, 2016

Could you write an accompanying test that proves that the hand-coded version returns the same answer as the BLAS version? Preferably use the example raised in the issue.

Also rebase your commits later.

@lokeshh
Copy link
Member Author

lokeshh commented Jan 25, 2016

I do not know what the function should do in case there's a complex number in the vector. For example if the vector is [1, 1+2i, 3], what should be the output?

From the BLAS version, it seems that it should be (12 + 12 + 32 + 2**2)0.5. I was wondering if there's some documentation just to be sure, so no corner cases are left.

@translunar
Copy link
Member

@lokeshh It is defined here, in the comments: http://for2html.sourceforge.net/BLAS/dznrm2.html

#  DZNRM2 returns the euclidean norm of a vector via the function
#  name, so that
#
#     DZNRM2 := sqrt( conjg( x' )*x )

x = NMatrix.new([4,1], [2,-4,3,5], dtype: dtype)
nrm2 = 5.385164807134504
end

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this look good?

translunar added a commit that referenced this pull request Feb 1, 2016
Fix cblas_nrm2 for complex types (fixes #389)
@translunar translunar merged commit e072306 into SciRuby:master Feb 1, 2016
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

3 participants