Skip to content

Fixed issues with the #have_func calls and the "unused variable" warnings #238

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

Merged
merged 6 commits into from
May 14, 2014

Conversation

andrewcsmith
Copy link

There was an error in the #have_func calls in extconf.rb, where the type of the function wasn't included.

There was also a warning that kept popping up because of unused variables. This popped up every time I ran my tests when including NMatrix.

lib/nmatrix/nmatrix.rb:566:
warning: assigned but unused variable - new_cap

lib/nmatrix/lapack.rb:196:
warning: assigned but unused variable - result
@@ -172,17 +172,17 @@ def gplusplus_version #:nodoc:
# export CPLUS_INCLUDE_PATH=/usr/local/atlas/include
# (substituting in the path of your cblas.h and clapack.h for the path I used). -- JW 8/27/12

idefaults = {lapack: ["/usr/include/atlas"],
idefaults = {lapack: ["/usr/local/atlas/include"],
Copy link
Member

Choose a reason for hiding this comment

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

Is this definitely safe to substitute on all systems? Perhaps add the new path to the front of the array instead of changing it? Unless the point is that having the original one at all breaks things?

@translunar
Copy link
Member

This seems to work for me on Mac OS X. Here's my output from extconf:

 bundle exec rake compile
mkdir -p tmp/x86_64-darwin13.0.0/nmatrix/2.0.0
cd tmp/x86_64-darwin13.0.0/nmatrix/2.0.0
/usr/local/var/rbenv/versions/2.0.0-p247/bin/ruby -I. ../../../../ext/nmatrix/extconf.rb
checking for apparent GNU g++ binary with C++0x/C++11 support... 4.9
using C++ standard... c++11
g++ reports version... 4.9.0
checking for main() in -lclapack... yes
checking for main() in -llapack... yes
checking for main() in -lcblas... yes
checking for main() in -latlas... yes
checking for atlas/cblas.h... no
checking for cblas.h... yes
checking for clapack.h... no
checking for clapack_dgetrf() in cblas.h,clapack.h... no
checking for clapack_dgetri() in cblas.h,clapack.h... no
checking for dgesvd_() in clapack.h... no
checking for cblas_dgemm() in cblas.h... yes
creating nmatrix_config.h
creating Makefile
cd -
cd tmp/x86_64-darwin13.0.0/nmatrix/2.0.0
make

I don't see any problems merging this since it's passing Ubuntu and passing Mac separately. Thanks for the excellent patch and great research.

translunar added a commit that referenced this pull request May 14, 2014
Fixed issues with the #have_func calls and the "unused variable" warnings
@translunar translunar merged commit b44773c into SciRuby:master May 14, 2014
@translunar
Copy link
Member

Okay, wait, now I'm confused. Why did you remove the function types after adding them in?

@andrewcsmith
Copy link
Author

Check out the long conversation here: #238

@cjfuller pointed out that the C code generated by int clapack_dgetrf was actually creating a function, which is why it didn't throw an error. It thinks it found the function, but actually it just successfully created one with a different signature. So I think that have_func in this context just doesn't work at all. I can't figure out how to get it to work.

Anyway, thanks for merging, and I think more research and tinkering is in order before we can get have_func to work and be useful. Also, what is the output of have_func actually used for in the greater NMatrix library? It's supposed to #define HAVE_CLAPCK_DGETRF as a pre-processor macro, but I don't see that we even reference that macro in the code.

@translunar
Copy link
Member

We don't reference the macro because I couldn't make it work. In some cases I have native versions of functions that could be called when ATLAS is unavailable, for example. And some ATLAS builds lack certain functions that others have.

@andrewcsmith andrewcsmith deleted the fixing_warnings branch May 15, 2014 01:57
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.

3 participants