Ruby Wrappers for NTheory #38

rajithv · 2016-05-12T15:03:59Z

No description provided.

rajithv · 2016-05-12T15:05:12Z

ext/symengine/ruby_ntheory.c

+    VALUE result;
+
+    unsigned long cbasic_operand1;
+    cbasic_operand1 = NUM2ULONG(operand1);


@abinashmeher999 am I using this correctly? Pls check.

Yes, that is correct. If you get unsure about return types, refer here: http://rxr.whitequark.org/mri/ident

Then all you need to do is to add an implicit conversion of SymEngine::Integer into Integer

I figured our half way on how to do this:

so I added a new method to the 'Integer' class
rb_define_method(c_integer, "to_int", cinteger_to_int, 0);

From which I should return a VALUE pointer pointing to a Ruby Bignum or a Fixnum. This can be done by converting a C int by using INT2NUM macro.

The step I'm stuck at is on extracting a C int from the SymEngine Integer object. @abinashmeher999 can you point me to somewhere I can read more about the Ruby Wrapper's SymEngine Integer's structure?

@rajithv, the Integer object uses GMP underneath. So you can extract a gmp int easily using the as_mpz() method. If you want an integer, you can call as_int(), but it will raise an exception if the integer doesn't fit into signed long int.

You can convert a SymEngine::Integer to a Ruby string first and then call to_i method

+1. This can be done. So instead of using the Ruby C API for the above, you can define #to_int in ruby itself here. Make a new file integer.rb and then proceed.

abinashmeher999 · 2016-05-13T09:34:36Z

spec/ntheory_spec.rb

+    end
+
+    describe '#mod' do
+      context '5 mod 4' do


Also add tests for other permutations of a positive and a negative number and see if they behave the same way as ruby, because I have heard they do not behave the same way in python and C.

@abinashmeher999 The same happens in Ruby. So I should adapt the code to return Ruby-like answers. right?

IMHO symengine's behavior should be consistent across all languages. But again there shouldn't be any surprises for ruby users. If both are same then it's good.

I agree with the 2nd not the 1st point.
There are two versions, mod and mod_f and they are used in C++ and Python respectively. You want the mod_f version for Ruby

I modified the ruby wrapper to return the same as mod_f.
But should I go ahead and wrap mod_f in SymEngine, and get that to the ruby wrapper? Or is it not necessary?

Also, should there be a way to access the c-style mod operation in Ruby?

It'll be better to have the mod_f function in SymEngine. For now, that's okay. You can open an issue to keep track of it.

rajithv · 2016-05-13T15:17:39Z

@abinashmeher999 @isuruf good to merge?

abinashmeher999 · 2016-05-13T16:14:55Z

ext/symengine/ruby_ntheory.h

+#include "symengine_utils.h"
+
+VALUE cntheory_onearg(void (*cwfunc_ptr)(basic_struct*, const basic_struct*), VALUE operand1);
+VALUE cntheory_twoarg(void (*cwfunc_ptr)(basic_struct*, const basic_struct*, const basic_struct*), VALUE operand1, VALUE operand2);


Please break this one too.

It's actually not needed. Removed it.

abinashmeher999 · 2016-05-13T16:15:25Z

Let's wait for the tests to pass.

isuruf · 2016-05-13T16:28:49Z

spec/ntheory_spec.rb

+    describe '#mod' do
+      context '5 mod 4' do
+        it 'returns 1' do
+          f = SymEngine::mod(5, 4)


This should use ruby's % operator

So I should overload % right? not introduce a new method as mod.

isuruf · 2016-05-14T09:25:50Z

spec/ntheory_spec.rb

      context '5 mod 4' do
        it 'returns 1' do
-          f = SymEngine::mod(5, 4)
+          f = 5 % 4


You are testing ruby's in built ints here

rajithv · 2016-05-17T06:14:24Z

@abinashmeher999 @isuruf pls review.

isuruf · 2016-05-17T06:18:57Z

spec/ntheory_spec.rb

+    describe '#fibonacci' do
+      context '5th Fibonacci Number' do
+        it 'returns 5' do
+          f = SymEngine::fibonacci(5)


You should add another test which uses a SymEngine integer instead of Ruby's

Better to add it to all of them. Or at least one for one argument and one for two argument function.

rajithv · 2016-05-17T09:38:37Z

@isuruf @abinashmeher999 done

abinashmeher999 · 2016-05-17T10:12:04Z

spec/ntheory_spec.rb

+      context '-5 mod 4' do
+        it 'returns 3' do
+          f = @im5 % @i4
+          expect(f).to eql(3)


So, f is a SymEngine::Integer, right? Will implicit conversion happen here while comparing?

I just found eql checks hash equality and type conversions are taken care in eq.

abinashmeher999 · 2016-05-17T10:20:34Z

spec/ntheory_spec.rb

+          expect(f).to eql(5)
+        end
+      end
+      context 'binomial (n=5, k=1)' do


Make the wording of the context like with SymEngine::Integer as parameters and same for the test above this with ruby Integer as parameters. This is easier to read when tests are run in verbose mode. Currently it will print

#binomial binomial (n=5, k=1) returns 5 binomial (n=5, k=1) returns 5

Accordingly modify it string. If the tests pass please make sure to make these changes in a new PR.

abinashmeher999 · 2016-05-17T10:43:09Z

@rajithv Thanks for the PR and your patience. +1 to merge. The minor changes can be incorporated in another PR.

rajithv added 4 commits May 12, 2016 12:24

Added NTheory methods to Binding List

d56b892

NTheory Wrappers

5ad2393

Next Prime, GCD, LCM, mod and quotient working

6cd53d1

Ntheory functions wrapped

d57e5f5

rajithv reviewed May 12, 2016
View reviewed changes

rajithv added 2 commits May 13, 2016 09:13

Restructuring macros->utils

94614ce

Tests Done

719470c

abinashmeher999 reviewed May 13, 2016
View reviewed changes

Fixed incompatibility of mod operation

515b715

rajithv changed the title ~~[WIP] Ruby Wrappers for NTheory~~ Ruby Wrappers for NTheory May 13, 2016

abinashmeher999 reviewed May 13, 2016
View reviewed changes

isuruf reviewed May 13, 2016
View reviewed changes

changed mod -> %

6b29a73

isuruf reviewed May 14, 2016
View reviewed changes

isuruf reviewed May 17, 2016
View reviewed changes

SymEngine::Integer to int implicit conversion

f29f4c4

rajithv force-pushed the ntheory branch from 7a8a42e to f29f4c4 Compare May 17, 2016 09:29

abinashmeher999 reviewed May 17, 2016
View reviewed changes

isuruf merged commit 7ea8a63 into symengine:master May 17, 2016

Ruby Wrappers for NTheory #38

Ruby Wrappers for NTheory #38

Uh oh!

Conversation

rajithv commented May 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajithv May 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajithv commented May 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abinashmeher999 commented May 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajithv commented May 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajithv commented May 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abinashmeher999 May 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abinashmeher999 commented May 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rajithv May 13, 2016 •

edited

Loading

abinashmeher999 May 17, 2016 •

edited

Loading