Skip to content

Use symengine's new interface for lambda double #106

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

Merged
merged 2 commits into from
Oct 30, 2016
Merged

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Oct 17, 2016

@isuruf
Copy link
Member Author

isuruf commented Oct 18, 2016

Ready for review. LLVMDoubleVisitor should be another PR.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good, thanks!

@bjodah
Copy link
Contributor

bjodah commented Oct 18, 2016

Looks great!

@isuruf
Copy link
Member Author

isuruf commented Oct 19, 2016

Tests fail with
symengine.tests.test_lambdify.test_cse_list_input ... bin/test_travis.sh: line 19: 22264 Segmentation fault: 11 nosetests -v
I'll try to reproduce this locally

@bjodah
Copy link
Contributor

bjodah commented Oct 19, 2016

Do you think #105 might be related?

@isuruf
Copy link
Member Author

isuruf commented Oct 19, 2016

I just pushed a fix, let's see.

@sighingnow
Copy link

Hi @isuruf I'm afraid that simplify expressions before sending to sympy can't fix #105. In the implementation of sympy's cse function, line 530 of cse_main.py, we can see that all expressions must be instances of sympy's Basic class. However, we can't inherit symengine's Basic class from sympy's Basic class, as cdef classes can't inherit from normal Python classes, see cython doc. So, just simplify the expressions can't fix #105.

I think the correct way to fix it is casting expression to sympy's value before sending to the sympy.cse function. I mean, something like this:

    subs, new_exprs = cse([expr._sympy_() for expr in exprs])

@isuruf
Copy link
Member Author

isuruf commented Oct 19, 2016

@sighingnow, sympy's sympify call Basic._sympy_(). So, it's the same thing

@sighingnow
Copy link

OK, I got it. Thanks very much for your reply @isuruf .

@isuruf
Copy link
Member Author

isuruf commented Oct 23, 2016

Anybody have access to a Mac?

@isuruf
Copy link
Member Author

isuruf commented Oct 23, 2016

On OS X, Python=3.5 Release mode fails while Debug mode passes

@bjodah
Copy link
Contributor

bjodah commented Oct 23, 2016

I have ssh access to a Mac OS X machine, never used OX X though, I will try to debug.
EDIT: The Debug mode build on osx uses Python 2.7 as far as I can tell, no Python 3.5 build on OSX with Debug mode enabled?

@bjodah
Copy link
Contributor

bjodah commented Oct 23, 2016

I'm struggling on OSX, I can't even get libsymengine ctest suite to pass, all tests fail like this:

Test project /Users/user/isuruf-llvm/symengine.py-llvm/symengine-cpp/build
      Start  1: test_rcp
 1/34 Test  #1: test_rcp .........................***Exception: Other  0.07 sec
...
The following tests FAILED:
      1 - test_rcp (OTHER_FAULT)
...

@isuruf
Copy link
Member Author

isuruf commented Oct 23, 2016

Hmm, can you try the conda package for symengine?

@bjodah
Copy link
Contributor

bjodah commented Oct 23, 2016

I will, but I'm afraid I've run out of time for today. I can try again next weekend. If you want to make progress earlier I could try to provide you with an account on that box with ssh access? Just email me your public ssh key or preferred password to my gmail: bjodah
and I'll try to set it up tomorrow.

@isuruf
Copy link
Member Author

isuruf commented Oct 24, 2016

Thanks to @bjodah, I got access to a Mac and I was able to reproduce this.
Python 3.5 in Release mode with AppleClang triggers this segfault. Segfault is not there when a different Python version is used or in Debug mode or with gcc.
All the lambdify tests segfault and the segfault happens in init, before the __call__.
I'll update this thread, when I find something more.

This fixes a segfault in lambdify. This segfault is probably a
a Cython bug or a compiler bug or a combination of both
@isuruf
Copy link
Member Author

isuruf commented Oct 29, 2016

Added a fix. I'm not happy with the fix, but this is the best I could come up for now. I'll try to get a proper fix in later. Meanwhile, can somebody review the last commit , so that we can move on the LLVMDoubleVisitor?

@certik
Copy link
Contributor

certik commented Oct 29, 2016

I think this is fine.

In the long run, I still think we should switch our Python wrappers to use the C API. Still use Cython, but just a C compiler and C API. This will speedup the compilation greatly, and get rid of all these bugs. We already have an issue for this at #91.

@certik
Copy link
Contributor

certik commented Oct 29, 2016

+1 to merge as it is.

@isuruf isuruf merged commit d38a3f1 into symengine:master Oct 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants