Complex class #39

abinashmeher999 · 2016-05-24T06:19:10Z

@rajithv Please open up a WIP PR from now on whenever you start working on something. That way if you run into a problem, all the conversations can happen there.

abinashmeher999 · 2016-05-24T06:32:42Z

ext/symengine/ruby_complex.c

+    imag = rb_funcall(comp_value, rb_intern("imaginary"), 0, NULL);
+
+    crational_init(real_basic, real);
+    crational_init(imag_basic, imag);


crational_init takes a VALUE and extracts the c pointer from that in this line. You are passing a basic_struct * which is being cast to VALUE/unintptr_t/void *.

I assume you want to extract the numerator and denominator info from ruby Rational objects so that you can pass it to complex_set. You can use sympify to do that.

@abinashmeher999 I realised that this is wrong and am looking to solve the issue with sympify. This would solve the issue with Rational as well.

rajithv · 2016-05-24T17:40:41Z

@abinashmeher999 @isuruf

The sympify function has been exposed, but there's a small error. I am guessing it's a result of mismanaging memory, but cannot seem to find out what's causing the error. Probably something small. I've been trying for a long time now, without success.

Basically, this is what happens now:

2.1.8 :002 > a = Rational('1/2')
 => (1/2) 
2.1.8 :003 > a1 = SymEngine::sympify(a)
 => #<SymEngine::Rational:0x92831c0> 
2.1.8 :004 > a1.class
 => SymEngine::Rational 
2.1.8 :005 > a1.to_s
(irb):5: [BUG] Segmentation fault at 0x000001
ruby 2.1.8p440 (2015-12-16 revision 53160) [i686-linux]

and

2.1.8 :001 > a = 100
 => 100 
2.1.8 :002 > b = 200
 => 200 
2.1.8 :005 > a1 = SymEngine::sympify(a)
 => #<SymEngine::Integer:0x8a8f374> 
2.1.8 :006 > b1 = SymEngine::sympify(b)
 => #<SymEngine::Integer:0x8a83fd8> 
2.1.8 :007 > c1 = a1 + b1
terminate called after throwing an instance of 'std::logic_error'
  what():  /home/rajith/Development/symengine/symengine/utilities/teuchos/Teuchos_RCPNode.hpp:95:

Throw number = 1

Throw test that evaluated to true: true

Any insights into solving this issue?

isuruf · 2016-05-24T17:46:31Z

ext/symengine/ruby_utils.c

+
+    VALUE result;
+    basic cbasic_operand;
+    basic_new_stack(cbasic_operand);


basic_new_stack -> basic_new_heap

it did not work.. actually now the error comes earlier

2.1.8 :002 > a = Rational('1/2') => (1/2) 2.1.8 :003 > a1 = SymEngine::sympify(a) terminate called after throwing an instance of 'std::logic_error' what(): /home/rajith/Development/symengine/symengine/utilities/teuchos/Teuchos_RCPNode.hpp:95:

@isuruf thanks. It's working now.

symengine#21

isuruf · 2016-05-24T20:42:16Z

ext/symengine/symengine_utils.c

+            sympify(real, real_basic);
+            sympify(imag, imag_basic);
+
+            complex_set(cbasic_operand2, real_basic, imag_basic);


complex_set requires integers or rationals, but Ruby complex can have any real number as real and imaginary parts.
Best way is to do cbasic_operand2 = real_basic + basic_const_I * imag_basic

rajithv · 2016-05-25T10:51:14Z

@abinashmeher999 @isuruf

Shall we do the Complex Doubles and Complex MPCs in another PR? I am looking into complex doubles now, and it will need wrapping real values into Ruby as well. So I will add more tests for the SymEngine::Complex here, and then move on to another PR.

If there is anything else that needs to be done here, let me know.

isuruf · 2016-05-25T10:55:38Z

ext/symengine/symengine_utils.c

+
+
+            basic_struct *const_I = basic_new_heap();
+            basic_struct *imag_part = basic_new_heap();


You can use cbasic_operand2 instead of the new variables.

abinashmeher999 · 2016-05-25T11:21:42Z

We will also need a #to_c method for implicit conversion to Complex. Shouldn't take long.

v0dro · 2016-05-27T16:18:21Z

spec/complex_spec.rb

+        it 'returns an instance of SymEngine::Integer class' do
+          a = Complex(2, 7)
+          a = SymEngine::S(a)
+          a = a.real


It's bad practice to re-assign variables in side a single unit test. r = a.real would be better.

v0dro · 2016-05-27T16:20:56Z

One thing that I notice is that you're following 4 space tabs everywhere. Is that a convention for symengine in general? In Ruby we generally use a 2 space tab.

rajithv · 2016-05-27T17:00:23Z

I have been following the same convention as per the existing repository. But generally in the repo the Ruby files have a 2 space tab. And C files have a 4 space tab. Maybe @abinashmeher999 and @isuruf can give more detailed answers on these.

…o doubles Conflicts: spec/constant_spec.rb spec/rational_spec.rb

Conflicts: symengine_version.txt

…o complex

isuruf · 2016-05-28T10:19:37Z

spec/constant_spec.rb

+  describe '#i' do
+    subject(:i) { SymEngine::I }
+
+    it { is_expected.to be_a SymEngine::Constant }


Is this passing? I is not a constant, it's a Complex

rajithv · 2016-05-28T11:37:22Z

@abinashmeher999 @isuruf please review

zverok · 2016-05-28T11:41:57Z

lib/symengine/complex.rb

+module SymEngine
+  class Complex
+    def to_c
+        self.to_s.sub('I', 'i').sub('*','').gsub(' ','').to_c


unnecessary self;

tr is better than sub in this case: tr('I', 'i').tr('* ', '')

isuruf · 2016-05-28T15:10:13Z

ext/symengine/ruby_complex.c

+
+    basic_free_stack(cbasic_operand);
+
+    return result;


Use function_onearg instead of the above implementation.

isuruf · 2016-05-28T16:17:59Z

+1 to merge, @zverok, any other comments?

zverok · 2016-05-28T16:48:15Z

lib/symengine/complex.rb

+module SymEngine
+  class Complex
+    def to_c
+        to_s.tr('I', 'i').tr('*','').tr(' ','').to_c


This is pretty minor, but you can use one tr('* ', '') instead of two, when replacing chars with nothing.
Oh! Last moment thought: just String#delete would be even better :)

zverok · 2016-05-28T16:48:52Z

Only one last (and pretty minor). Otherwise everything looks good to me 👍

rajithv · 2016-05-28T17:30:42Z

Thanks for the insight @zverok

rajithv added 3 commits May 23, 2016 20:35

Basic structure for complex

f9b0661

Rational spec changed to reflect required changes

b4af3b0

Adding spec for Complex

316a9c1

abinashmeher999 reviewed May 24, 2016
View reviewed changes

Exposed sympify to Ruby

88b302f

rajithv force-pushed the complex branch from 4a172d0 to 88b302f Compare May 24, 2016 17:03

isuruf reviewed May 24, 2016
View reviewed changes

Expose sympify to Ruby - correction

dbd072d

rajithv force-pushed the complex branch from 5769be4 to dbd072d Compare May 24, 2016 18:06

rajithv added 5 commits May 24, 2016 23:50

Added complex to klass of basic & sympify

d1498f4

Removed inits of Complex and Rational. This fixes symengine#19 and fixes

62d67ed

symengine#21

Rational Spec updated to replace .new with Sympify

505618a

Ruby wrapper for i, the constant

dd79ff0

Complex numbers done

0f4694a

isuruf reviewed May 24, 2016
View reviewed changes

rajithv added 2 commits May 25, 2016 13:32

Edited complex to accept not only rational

6633cb4

Tests edited to support versions <= 2.0

cb28383

isuruf reviewed May 25, 2016
View reviewed changes

rajithv added 2 commits May 25, 2016 16:56

Minor corrections

fa1fbe5

to_c method

7def8b6

rajithv added 2 commits May 26, 2016 18:08

renamed sympify->convert and short form to SymEngine()

038aace

SymEngine version update

a4b43d4

v0dro reviewed May 27, 2016
View reviewed changes

rajithv added 5 commits May 28, 2016 10:28

SymEngine version update

a219d58

Merge branch 'master' of http://github.com/symengine/symengine.rb int…

7953c83

…o doubles Conflicts: spec/constant_spec.rb spec/rational_spec.rb

Merge branch 'doubles' into complex

417eb97

Conflicts: symengine_version.txt

Merge branch 'master' of http://github.com/symengine/symengine.rb int…

1246cb2

…o complex

Complex Spec

81ade4c

rajithv force-pushed the complex branch from 7bab4de to 81ade4c Compare May 28, 2016 10:02

isuruf reviewed May 28, 2016
View reviewed changes

rajithv added 2 commits May 28, 2016 16:05

Tests for convert() and SymEngine()

e9162e0

Changed I from Constant to Complex

7ddd68e

zverok reviewed May 28, 2016
View reviewed changes

Fixing errors

e96d934

isuruf reviewed May 28, 2016
View reviewed changes

Fixing further errors

52e1870

zverok reviewed May 28, 2016
View reviewed changes

minor change

5802cf7

isuruf changed the title ~~[WIP] Complex class~~ Complex class May 29, 2016

isuruf merged commit 6d9b596 into symengine:master May 29, 2016



		basic_struct *const_I = basic_new_heap();
		basic_struct *imag_part = basic_new_heap();

Complex class #39

Complex class #39

Uh oh!

Conversation

abinashmeher999 commented May 24, 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 commented May 24, 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 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abinashmeher999 commented May 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

v0dro commented May 27, 2016

Uh oh!

rajithv commented May 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajithv commented May 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isuruf commented May 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zverok commented May 28, 2016

Uh oh!

rajithv commented May 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants