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

Complete compatibility with nmatrix and new release as an nmatrix extension. #23

Closed
v0dro opened this issue Nov 7, 2015 · 9 comments
Closed

Comments

@v0dro
Copy link
Member

v0dro commented Nov 7, 2015

Updating rb-gsl to comply with the latest nmatrix in all methods will increase interoperability and let users take advantage of other nmatrix extensions as well.

Also, I'd suggest releasing a new gem 'nmatrix-gsl' as an nmatrix extension, that will load the NMatrix class with methods that will call GSL internally, similar to nmatrix-atlas or nmatrix-lapacke. The only hurdle with the extension approach being that of maintaining the methods that will be loaded into nmatrix.

One way I think this can be implemented is by keeping nmatrix-gsl in the main nmatrix repo just like the other nmatrix extensions and adding rb-gsl as a dependency to the nmatrix-gsl gem.

But the problem would be maintaining rb-gsl and nmatrix-gsl in parallel. API changes to rb-gsl will require changes in nmatrix-gsl (in a different repo) as well.

@MohawkJohn @minad @agarie looking forward to hear your thoughts on this.

@minad
Copy link
Contributor

minad commented Nov 7, 2015

I am a bit unsure since the nmatrix/narray code is intertwined in the gsl calls. A while ago I tried to reduce that a bit, however I don't think it is really possible as a separate gem.

I would rather concentrate on gsl2.0, tests and nmatrix compatibility instead of restructuring.

@v0dro
Copy link
Member Author

v0dro commented Nov 9, 2015

Yes the bit about gsl2.0 is what I'll be doing. That's my main focus.

My question is where should the nmatrix-gsl extension gem be placed. Withing rb-gsl or within nmatrix?

@minad
Copy link
Contributor

minad commented Nov 9, 2015

I think it is not necessary to have a separate nmatrix-gsl gem. I would just try to adapt rbgsl to make it compatible with nmatrix as it stands.

Which means - the code goes into the rbgsl repository.

@v0dro
Copy link
Member Author

v0dro commented Jan 3, 2016

@minad so here's my plan for full nmatrix compatibility of GSL:

  • In every test file that is currently housed in the test/gsl folder, write an additional function or set of functions that will take an NMatrix as input and return an NMatrix as output for whatever functions are being tested in the file. Something similar to this NArray test that is already present.
  • These tests will only be written for functions that actually compute something, and for conversion between GSL and NMatrix data types (for example, from GSL::Matrix to NMatrix).
  • Once the tests have been written, I will start modifying the code so that the tests pass.
  • When all the tests are passing, I will begin with fixing bugs relating to working with different GSL versions.
  • Support for 1.15 will be dropped, and rb-gsl will support GSL 2.1 and 1.16 once the whole project is over.
  • For accomplishing backwards compatibility with different GSL versions, guards will be placed in the C code.

I'll link you to my work as soon as I start.

Eager to hear your thoughts on this.

@minad
Copy link
Contributor

minad commented Jan 4, 2016

@v0dro Sounds like a plan - I like your test driven approach! Concerning your first point - maybe this could be generated (e.g. something like output = fn(input) vs output = fn(input.to_nmatrix).from_nmatrix). About the different versions, I think I mentioned it before - we should at least support the widely deployed gsl versions.

@v0dro
Copy link
Member Author

v0dro commented Jan 9, 2016

Could you elaborate on what exactly you mean by generated?

Maybe give me an example? Consider this code:

  def test_dht3
    vin = GSL::Vector.alloc(N)
    dht = GSL::Dht.alloc(N, 1.0, 20.0)

    N.times { |i|
      x = dht.x_sample(i)
      vin[i] = Math.exp(-x)
    }

    vout = dht.apply(vin)

    assert_equal 0.18148296716239096,   vout[0]
    assert_equal 0.35680451269699853,   vout[5]
    assert_equal 0.21101009980456306,   vout[10]
    assert_equal 0.02892068100516861,   vout[35]
    assert_equal 0.0022121119664674426, vout[100]
  end

How would you generate nmatrix tests from this? What about applying said approach to whole files?

@minad
Copy link
Contributor

minad commented Jan 9, 2016

I didn't think much about it. I would just prefer if you can write the tests somehow generically in such a way that you don't duplicate all the cases for plain gsl/nmatrix/narray.

@v0dro
Copy link
Member Author

v0dro commented Jan 9, 2016

So I'm passing the different inputs in an Array and calling #each on it so that its passed to the respective functions.

For example: https://github.com/v0dro/rb-gsl/blob/nmatrix_compatibility/test/gsl/dht_test.rb#L7

ktns pushed a commit to ktns/rb-gsl that referenced this issue Feb 26, 2016
don't merge yet - reduce number of HAVE_NARRAY_H occurences
@v0dro
Copy link
Member Author

v0dro commented Mar 11, 2016

#33

@v0dro v0dro closed this as completed Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants