These are chat archives for symengine/symengine

25th
May 2015
Sumith Kulal
@Sumith1896
May 25 2015 09:08
Hi @shivamvats,
The updates regarding the Polynomial class is that:
  • test_polynomial.cpp is sent in, could you check on that?
  • I have integrated with Visitor pattern with not much thought, just as to not break the builds.
  • Made use of map_uint_mpz instead of map_uint_integer.
We need to discuss the next tasks in hand.
I am currently implementing the printer, then tests for add_poly and printer. Shouldn't take long.
Sumith Kulal
@Sumith1896
May 25 2015 09:13

Would this replacement for add_poly work?

    RCP<const Polynomial> add_poly(const Polynomial &a, const Polynomial &b) {
        map_uint_mpz dict;
        for(auto &it : a.dict_)
            dict[it.first] = it.second;
        for(auto &it : b.dict_)
            dict[it.first] += it.second;

        RCP<const Polynomial> c = polynomial(a.var_, std::move(dict));
        return c;
    }

@certik seems to have efficiency concerns there, so I am considering a replacement.

Shivam Vats
@shivamvats
May 25 2015 09:16
Let me see
Are you sure this is a correct replacement?
Sumith Kulal
@Sumith1896
May 25 2015 09:20
dict[it.first] is by default zero, if not initialized before.
Still I would like to test and only then change
Shivam Vats
@shivamvats
May 25 2015 09:21
Oh! Is it?
I didn't know that.
Sumith Kulal
@Sumith1896
May 25 2015 09:21
I had problem will writing test_add_poly()
void test_add_poly(){
    std::string x = "x";
    map_uint_mpz adict = {{0, 1}, {1, 2}, {2, 1}};
    map_uint_mpz bdict = {{0, 2}, {1, 3}, {2, 4}};

    RCP<const Polynomial> a = polynomial(x, std::move(adict));
    RCP<const Polynomial> b = polynomial(x, std::move(bdict));

    RCP<const Polynomial> c = add_poly(a, b);

    assert(c->degree == 2);
    assert(c->var_ == "x");
}
Is there a mistake in here?
Shivam Vats
@shivamvats
May 25 2015 09:26
I'll need to look at it
Does your latest commit work?
Sumith Kulal
@Sumith1896
May 25 2015 09:28
Yes, the test_polynomial has just the constructor test
that is passing
This error would not have been when add_poly was using RCP arguments
I am doing a mistake with passing of variables, I need to figure it out.
Shivam Vats
@shivamvats
May 25 2015 09:29
I think you need to use const
Oh wait, you have used it.
Did you make any changes to add_poly?
Sumith Kulal
@Sumith1896
May 25 2015 09:30
No
Shivam Vats
@shivamvats
May 25 2015 09:31
Ok, I'll try to figure it out.
Sumith Kulal
@Sumith1896
May 25 2015 09:31
Wait let me check
If I had made any changes or not
No I haven't. You should still try though, something might have crept in.
Shivam Vats
@shivamvats
May 25 2015 09:35
I don't think you need to use RCP here
Try using const Polynomial poly1(str, std::move(dict));
Sumith Kulal
@Sumith1896
May 25 2015 09:38
Correct, Thanks
tests pass
How do I even test the whole dict?
Shivam Vats
@shivamvats
May 25 2015 09:41
What do you mean by that?
Sumith Kulal
@Sumith1896
May 25 2015 09:42
If you notice the test_add_poly(), it has
    assert(c->degree == 2);
    assert(c->var_ == "x");
I also want test for c->dict_
Shivam Vats
@shivamvats
May 25 2015 09:45
I'd rather suggest getting the printer ready and testing the output
Sumith Kulal
@Sumith1896
May 25 2015 09:46
Yeah, that's a good option
So should I get in my current test_add_poly() or keep it pending?
Shivam Vats
@shivamvats
May 25 2015 09:48
You can print the dict for testing locally.
You can push the tests after adiing printer
Sumith Kulal
@Sumith1896
May 25 2015 09:49
Okay
iirc dict.cpp already has a printing function I believe, can be used for testing purposes atleast
Any other thing you want to discuss about the class?
Shivam Vats
@shivamvats
May 25 2015 09:52
Yeah you can use that
I think we can discuss the changes as you push
Sumith Kulal
@Sumith1896
May 25 2015 09:53
Yes
Thanks, that's it for now I think.
Shivam Vats
@shivamvats
May 25 2015 09:54
Great
Abinash Meher
@abinashmeher999
May 25 2015 09:56
Hi guys!
Do you know if we are enforcing immutability with objects in symengine?
Abinash Meher
@abinashmeher999
May 25 2015 10:18
Also there's a line in cwrapper.h
typedef basic_struct basic[1];
And further we have referred to as basic. Not sure how this works. Is this a pointer to basic_struct? If yes we could have used
typedef basic_struct* basic;
Isuru Fernando
@isuruf
May 25 2015 11:39
@abinashmeher999, that's a trick from gmp library to pass by reference in C.
Isuru Fernando
@isuruf
May 25 2015 11:47
@Sumith1896 @shivamvats since you are introducing a new data structure called map_uint_mpz you should introduce a function to check for equality and to compare.
For example, map_basic_basic has a method to check equality, map_basic_basic_eq and a method to compare map_basic_basic_compare
Sumith Kulal
@Sumith1896
May 25 2015 11:50
I have noticed that only maps with Basic involved have these methods.
Hence I didn't implement when I introduced the map_unit_mpz.
Isuru Fernando
@isuruf
May 25 2015 11:52
If you are going to use them in different places (__eq__ method, tests, etc.) then you should have one function to do it
Sumith Kulal
@Sumith1896
May 25 2015 11:55
Okay, I didn't realise the value before, I'll do it.
Sumith Kulal
@Sumith1896
May 25 2015 12:07
Maybe that's the first thing I have to do now.
Abinash Meher
@abinashmeher999
May 25 2015 13:51
@isuruf I still don't get this. Can you direct me somewhere?
Abinash Meher
@abinashmeher999
May 25 2015 14:04
If that is the case will this work
basic result = (basic)malloc(sizeof(basic_struct));
Isuru Fernando
@isuruf
May 25 2015 14:09

yes, it should work, but you shouldn't allocate on the heap like that.

basic result;
basic_init(result);

is the recommended way

Abinash Meher
@abinashmeher999
May 25 2015 14:19

@isuruf the situation is somewhat like this

static VALUE cbasic_add(VALUE self, VALUE operand2){
    basic this, basic_operand2;
    basic result = (basic)malloc(sizeof(basic_struct));//here I am allocating on the heap
    basic_init(result);

    Data_Get_Struct(self, basic_struct, this);
    Data_Get_Struct(operand2, basic_struct, basic_operand2);

    basic_add(result, this, basic_operand2);

    return Data_Make_Struct(self, basic_struct, rb_gc_mark, cbasic_release, result);
}

I am wrapping result with ruby C API. We want the pointer to be valid outside the function. In that case I will need the pointer to be allocated on the heap. Or is there some similar workaroud?

Isuru Fernando
@isuruf
May 25 2015 15:00
I'm not sure. You'll have to work with the basic_struct. If you have time, take a look at the gmp ruby gem, https://github.com/srawlins/gmp/blob/master/ext/ruby_gmp.h
Abinash Meher
@abinashmeher999
May 25 2015 15:48
I have come across that earlier, but I haven't delved deeper into the structure. I will go through it. Thanks!
Isuru Fernando
@isuruf
May 25 2015 15:50
In GMP, mpz_t is an array of 1 MP_INT struct. They have used the struct MP_INT everywhere.
Abinash Meher
@abinashmeher999
May 25 2015 16:55
Thanks @isuruf ! :smiley: Now I know what to look for.