Wrap Miscellaneous Functions #172

ShikharJ · 2017-07-08T04:42:24Z

No description provided.

ShikharJ · 2017-07-08T23:52:31Z

symengine/lib/symengine_wrapper.pyx

+    cdef RCP[const symengine.Pow] a1 = symengine.rcp_static_cast_Pow(_a.thisptr)
+    return c2py(symengine.pow_expand(a1))
+
+expand_pow = pow_expand


@isuruf Can you tell me why the above code is failing? More specifically what can be done to prevent this:

symengine_wrapper.obj : error LNK2019: unresolved external symbol "class SymEngine::RCP<class SymEngine::Basic const > __cdecl SymEngine::pow_expand(class SymEngine::RCP<class SymEngine::Pow const > const &)" (?pow_expand@SymEngine@@YA?AV?$RCP@$$CBVBasic@SymEngine@@@1@ABV?$RCP@$$CBVPow@SymEngine@@@1@@Z) referenced in function "struct _object * __cdecl __pyx_pf_9symengine_3lib_17symengine_wrapper_36pow_expand(struct _object *,struct _object *)" (?__pyx_pf_9symengine_3lib_17symengine_wrapper_36pow_expand@@YAPAU_object@@PAU1@0@Z) [C:\projects\symengine-py\build\lib.win32-2.7\symengine\lib\symengine_wrapper.vcxproj]

The first question is, why do you need them?

Besides, pow_expand, mul_expand and add_expand should be removed from the public API

To cater to Sympy's expand_mul.

If we really needed that we can just do expand_mul = expand.

isuruf · 2017-07-09T04:32:59Z

symengine/lib/symengine_wrapper.pyx

@@ -2740,6 +2788,32 @@ def exp(x):
    cdef Basic X = sympify(x)
    return c2py(symengine.exp(X.thisptr))

+def isqrt(n):


Why do we need another function? Isn't sqrt enough?

isuruf · 2017-07-09T12:43:14Z

symengine/lib/symengine_wrapper.pyx

+            raise TypeError
+    except TypeError:
+        raise ValueError('%s is not an integer' % (n,))
+    return result


For this, you can use SymPy's as_int as it isn't SymEngine or SymPy specific.

isuruf · 2017-07-09T12:44:06Z

symengine/lib/symengine_wrapper.pyx

+    require(_n, Integer)
+    return symengine.perfect_power(deref(symengine.rcp_static_cast_Integer(_n.thisptr)))
+
+def perfect_square(n):


Sympy uses is_square

I would rename this to is_square instead of aliasing. Two names for the same method is needed only for sympy compatibility or backwards compatibility.

ShikharJ · 2017-07-11T14:28:08Z

@isuruf Can you review this?

isuruf · 2017-07-11T14:30:41Z

symengine/lib/symengine_wrapper.pyx

+        return (c2py(<RCP[const symengine.Basic]>_r), true)
+    return (c2py(<RCP[const symengine.Basic]>_r), false)
+
+integer_nthroot = i_nth_root


ShikharJ · 2017-07-14T04:56:00Z

Ping @isuruf.

isuruf · 2017-07-14T14:41:44Z

symengine/lib/symengine_wrapper.pyx

@@ -895,6 +901,20 @@ class Symbol(Basic):
        return self.__class__


+class Dummy(Symbol):


This class is not tested anywhere

isuruf · 2017-07-14T14:42:24Z

Can you add more tests?

ShikharJ · 2017-07-16T00:24:04Z

Ping @isuruf.

isuruf · 2017-07-16T04:13:08Z

symengine/__init__.py

@@ -9,7 +9,8 @@
        UndefFunction, Function, FunctionSymbol as AppliedUndef,
        have_numpy, true, false, Equality, Unequality, GreaterThan,
        LessThan, StrictGreaterThan, StrictLessThan, Eq, Ne, Ge, Le,
-        Gt, Lt, GoldenRatio, Catalan, EulerGamma)
+        Gt, Lt, GoldenRatio, Catalan, EulerGamma, Dummy, perfect_power,
+        is_square, integer_nthroot, isprime, sqrt_mod, igcdex)


Functions like is_square and igcdex are not pulled in when you do from sympy import *. I'd remove them from here.

isuruf · 2017-07-16T04:14:12Z

symengine/lib/symengine_wrapper.pyx

+    cdef int ret_val = symengine.i_nth_root(symengine.outArg_Integer(_r), deref(symengine.rcp_static_cast_Integer(_a.thisptr)), n)
+    if ret_val == 1:
+        return (c2py(<RCP[const symengine.Basic]>_r), true)
+    return (c2py(<RCP[const symengine.Basic]>_r), false)


return (c2py(<RCP[const symengine.Basic]>_r), ret_val == 1)

isuruf · 2017-07-16T04:15:51Z

symengine/tests/test_symbol.py

@@ -1,4 +1,5 @@
 from symengine import Symbol, symbols, symarray, has_symbol
+from symengine.lib.symengine_wrapper import Dummy


change this to import from symengine

isuruf · 2017-07-16T04:16:18Z

symengine/lib/symengine_wrapper.pyx

@@ -3103,6 +3151,8 @@ def probab_prime_p(n, reps = 25):
    require(_n, Integer)
    return symengine.probab_prime_p(deref(symengine.rcp_static_cast_Integer(_n.thisptr)), reps) >= 1

+isprime = probab_prime_p


This isn't tested

ShikharJ · 2017-07-16T05:23:48Z

Ping @isuruf.

ShikharJ force-pushed the Misc branch 3 times, most recently from c935090 to 80c48a7 Compare July 8, 2017 23:11

ShikharJ commented Jul 8, 2017

View reviewed changes

isuruf reviewed Jul 9, 2017

View reviewed changes

ShikharJ force-pushed the Misc branch 2 times, most recently from c762b0a to 53b6462 Compare July 9, 2017 22:13

ShikharJ changed the title ~~[WIP] Wrap Miscellaneous Functions~~ Wrap Miscellaneous Functions Jul 9, 2017

ShikharJ force-pushed the Misc branch from 53b6462 to e4ef6db Compare July 10, 2017 13:46

isuruf reviewed Jul 11, 2017

View reviewed changes

ShikharJ force-pushed the Misc branch 2 times, most recently from 2a90b0d to 4779a50 Compare July 13, 2017 18:13

isuruf reviewed Jul 14, 2017

View reviewed changes

ShikharJ force-pushed the Misc branch from 4779a50 to 4931e08 Compare July 14, 2017 19:13

isuruf reviewed Jul 16, 2017

View reviewed changes

Wrapped miscellaneous functions

81104e2

ShikharJ force-pushed the Misc branch from 4931e08 to 81104e2 Compare July 16, 2017 04:49

isuruf merged commit 3541ffc into symengine:master Jul 16, 2017

ShikharJ deleted the Misc branch July 16, 2017 14:48

		@@ -895,6 +901,20 @@ class Symbol(Basic):
		return self.__class__


		class Dummy(Symbol):

		@@ -1,4 +1,5 @@
		from symengine import Symbol, symbols, symarray, has_symbol
		from symengine.lib.symengine_wrapper import Dummy

Wrap Miscellaneous Functions #172

Wrap Miscellaneous Functions #172

Uh oh!

Conversation

ShikharJ commented Jul 8, 2017

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!

ShikharJ commented Jul 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShikharJ commented Jul 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isuruf commented Jul 14, 2017

Uh oh!

ShikharJ commented Jul 16, 2017

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!

ShikharJ commented Jul 16, 2017

Uh oh!

Uh oh!