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

ArrayFire bindings for Linear Algebra #3

Merged
merged 125 commits into from
Sep 4, 2017
Merged

Conversation

prasunanand
Copy link
Member

@prasunanand prasunanand commented Aug 17, 2017

Implemented following classes:

  1. Af_Array
  2. Algorithm
  3. Arith
  4. BLAS
  5. LAPACK
  6. CUDA
  7. OpenCL
  8. Device
  9. Random
  10. Sparse
  11. Statistics

Added exception handling.
Only doubles implemented currently

RDoc::Task.new do |rdoc|
rdoc.main = "README.md"
rdoc.rdoc_files.include(%w{README.md LICENSE CONTRIBUTING.md lib ext})
end
Copy link

Choose a reason for hiding this comment

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

Will this task run if you do a gem build on the gemspec? Is it not possible to add this to the gemspec itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

@v0dro I didn't get the question. Why would we need rdoc to run when using gem build.?

AFAIK, RubyDoc.info automatically generates the docs when I push the gem.

Copy link

Choose a reason for hiding this comment

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

So the above rake commands adds rdoc files which are then read by rubygems.org to generate YARD docs. If its not done during gem build and if you forget to run above rake task, the rdoc files will not get added to the gemspec and rubygems wont be able to detect them.

Copy link
Member Author

@prasunanand prasunanand Sep 4, 2017

Choose a reason for hiding this comment

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

@v0dro, I will follow Victor's advice about using rubygems-tasks library.

return Data_Wrap_Struct(CLASS_OF(left_val), NULL, arf_free, result); \
}

#define DEF_UNARY_RUBY_ACCESSOR(oper, name) \
Copy link

Choose a reason for hiding this comment

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

Nice use of macros!

static VALUE arf_device_info(VALUE self, VALUE name_val, VALUE platform_val, VALUE toolkit_val, VALUE compute_val){
char* d_name = (char*)malloc(sizeof(char) * 64);
char* d_platform = (char*)malloc(sizeof(char) * 10);
char* d_toolkit = (char*)malloc(sizeof(char) * 64);
Copy link

Choose a reason for hiding this comment

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

Shouldn't you be using xmalloc and xfree everywhere? They're the functions from ruby.h and allocate memory that the ruby VM is aware. malloc will allocate from system heap not ruby VM heap.

@v0dro
Copy link

v0dro commented Sep 4, 2017

When do you plan to merge this?


af_get_dims(&dims[0], &dims[1], &dims[2], &dims[3], input->carray);

size_t* shape = (size_t*)malloc(ndims * sizeof(size_t));;
Copy link

Choose a reason for hiding this comment

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

Double colon at the 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.

Its outdated commit.

uint ndims;
af_get_numdims(&ndims, input->carray);

dim_t* dims = (dim_t*)malloc(ndims * sizeof(dim_t));
Copy link

Choose a reason for hiding this comment

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

its good practice in C to declare all the variables at the top of the function.

@prasunanand
Copy link
Member Author

@9prady9 You can merge this PR now :) .

@9prady9 9prady9 merged commit 522d238 into arrayfire:master Sep 4, 2017
@v0dro
Copy link

v0dro commented Sep 4, 2017

Is it possible to publish this using rubygems? Just gem install arrayfire-rb would be amazing.

@v0dro
Copy link

v0dro commented Sep 4, 2017

Also you should remove the statement saying that bindings aren't yet ready for production from README. And, add SciRuby to the acknowledgements at the bottom along with the website.

@9prady9
Copy link
Member

9prady9 commented Sep 4, 2017

@v0dro
I am not aware of rubygems, but in principle it should be possible. We have similar packaging in rust environment(rust package manager - cargo.io) known as crate. From the usage style, it seems like rubygems is similar to crates in Rust or javascript package manager - npm.

I don't think it is a good idea to remove the cautionary statements until all the feasible functionality from ArrayFire is ported over to the ruby bindings. For one, I don't think @prasunanand has worked on image processing functions yet. Also, I believe indexing hasn't been worked on either.

@v0dro
Copy link

v0dro commented Sep 4, 2017

Oh. Ok. @prasunanand can you create issues and set milestones for this purpose in the GitHub issue tracker? It will becomes easy for new contributors to quickly what's going on and how to contribute. Make things as granular as possible.

@9prady9 rubygems is the ruby packaging system and the central repository for most open source gems (yes, it is similar to npm and crates). Hosting the gem on rubygems will allow users to install with a single command instead of cloning and compiling.

@9prady9
Copy link
Member

9prady9 commented Sep 4, 2017

@prasunanand @v0dro Please use this issue to track progress of implementing left over features.

@prasunanand
Copy link
Member Author

Thanks @9prady9 for merging the PR.

@v0dro I am creating milestones and I will work on the missing features.

@v0dro
Copy link

v0dro commented Sep 5, 2017

Alright. I think you add them to @9prady9 's list.

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