-
Notifications
You must be signed in to change notification settings - Fork 115
#288 Expose c++ integrator to python and remove scipy dependence #290
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
Conversation
|
It ran successfully on my Mac OS X 10.7.4 and Linux, but it's failing the nosetests in 10.5.8: |
|
Reiko, can you check if the above python session for integrating x^2 works for you? Also, in the tests directory, you can try which might tell us something useful. |
|
Very nice! This compiles and all tests pass on my Mac and the Linux cluster at CMU. |
|
Thanks, Mike. This makes the NFW halo class much nicer to use (no scipy). Leave only one thing to desire: pre-lensing coordinates instead of post-lensing coordinates. I know how this can be done, but this will have wait a little until I can pick it up. |
|
Hi Mike: So, the int1d crashes: so I've tried gdb. I got a bunch of |
|
Reiko, the fact that it crashes at dyld_dyld_start sounds like maybe it's trying to use an old installation. So maybe scons got confused and didn't relink everything correctly. Could you try recompiling completely from scratch: |
|
Mike: |
…d a bunch of print statements to help diagnose Reiko's bus error. (#288)
|
Hmm. I'm not sure then... Try the version I just pushed. I made a python layer version of int1d (now called galsim.integ.int1d rather than just galsim.int1d) that provides an interface to the C++ layer version. I don't think that will solve your problem, but it lets me put print statements just before the call to the c++ layer version. I also added a bunch of print statement in the c++ layer. So hopefully we can find the exact line where the bus error is happening and maybe have a better guess as to what might be causing it. |
|
Oh, and to avoid nosetests swallowing the python output, better to check this in your own python session: should do the trick. |
|
Hi Mike. Looks good---it seems to pass all tests (but it has a spurious output): Thanks! |
|
Great. I wasn't actually expecting that to fix the problem. Merely to provide some information to help diagnose the problem. I wonder what aspect of what I changed was the cure... :) Anyway, I removed the print statements, so it's probably worth making sure it still works now. If so, I'd say we're good to go. |
|
I don't know why "python layer version of int1d" works either. Something that may be relevant, though, is that my Python2.7 was compiled with gcc 4.0.1, while I compile GalSim with gcc4.7.1. I've rerun all tests, and all tests pass. |
|
That explains why your print statements didn't actually go through (python and c++ linking different clibs often cause I/O clashes). But I don't know why it would have caused a bus error for the int1d stuff. Anyway, glad it works now. |
|
Any more comments about this one? Barney and Rachel (when you get back online), you might want to look over the diff again, in particular the new file integ.py which has a python doc string now. See if that looks ok. Also, when working on Reiko's problem, I ended up switching the python function from |
|
Hi Mike... I'll take a good look at this later tonight and make comments (if any seem necessary). Shouldn't take too long. |
galsim/integ.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a fix for what I'm guessing is a typo in the specification of default rel_err=1.e-12 for abs_err... Please check!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Thank you.
|
It all looks very good to me... The only additional thing, which I think could be considered optional but would certainly not hurt, would be some simple unit tests for the integrator in a standalone fashion. A couple of functions would do (maybe one simple, one a little more oscillatory and one with at least one infinite limit to test that our defaults are good). If you're busy I volunteer, too... (I also owe you some changes to INSTALL.md by the way describing the new fink pathway... It's on the list for tomorrow!) |
|
That sounds worthwhile. I never did get around to writing unit tests for it. So if you're volunteering... :) A starting point could be the IntExample.cpp file in the examples directory for a C++ unit test. It includes an infinite integral. It wouldn't be too hard to convert these to python. There isn't anything there that's oscillatory yet. |
|
OK, cool, I'll push some unit tests this afternoon! |
…ite and infinite limits, another quite a bit harder (more oscillatory) (#288)
|
Hi Mike, I've added a few short tests... A nice simple function and a rather harder one with many (ever more rapid as x increases) zero crossings, using three sets of finite and three sets of infinite limits. It's only four tests, so if you would like to see more let me know and I can do a few, but I think the behaviour is already relatively impressive. I also made an edit to the docstring of |
|
Hi Barney:
|
|
Hmmm, I just realised that test 2 was stupid as it didn't actually run any code that would need the runtime error. I'll be back with a better suggestion in a sec... |
|
It's ok Barney. I think the problem stems from g++ 4.7's ostringstream being incompatible with earlier versions (e.g. the one that python was compiled with). I was able to reproduce the problem by using g++ 4.7 for GalSim with my system python (which uses gcc 4.2). It may not be ostringstream specifically, but when I change how the IntFailure exception is created by removing the ostringstream stuff, then the exception passes through the python-c++ boundary correctly. I'll push the fix in a few minutes. |
|
OK, Reiko, give it a try now. |
|
OK... Go back to test 1, and try the following in the python interpreter: You should see something like... ...and an ugly crash. Now try the same thing in test 2 (test 3 should give the same results). In test 2's commit, I get (using 4.7.2 gcc-built galsim and gcc 4.0.1 EDP): ... Oh, I see Mike has just posted telling me not to worry! Well, maybe this will turn out useful... |
|
Sorry for the confusion Reiko, try |
|
Hi Mike & Barney, |
|
Running your tests, Barney, I get: (2) test2, same as above Could it be that I need to worry about the boost.python gcc compilation version too? |
|
Hmm.. That means that neither the try/catch block in pysrc/Integ.cpp, nor the boost.testing try/catch block are able to catch the exception. I found some articles about exceptions not being caught across shared libraries (thrown in one, caught in another), so I'll try to see what kind of fix is recommended to get that to work. |
|
Reiko, can you please post the results of these two commands: |
|
Sorry, the last post (deleted) was from my other laptop. The correct outputs are: |
|
Thanks. I think the problem is that you have two versions of libgcc_s in there. The gcc47 one is probably the one we want. Not sure why the other one is getting linked as well. Also, it's a bit strange that tmv isn't listed there. Can you post what command scons uses to link the library lib/libgalsim.0.dylib. (It should start with |
|
Is this it? |
|
Which thing required /usr/local/lib? Is that where TMV is installed probably? Also, /usr/local/lib is an odd place to have a gcc_s library. Do you have a version of gcc installed there? Anyway, my best guess as to what is going on is that having -L/usr/local/lib specified means that the linker includes that libgcc_s library, even though g++ later also adds its own libgcc_s library, and the two of them are conflicting in how they handle exceptions. I think the best solution is to figure out what installed a libgcc_s library in /usr/local/lib and uninstall it, since it's (probably) not the system gcc and it's not the MacPort gcc. So I don't think it's anything you are currently using. If that's not possible, then you can try installing TMV in a different location so TMV_DIR won't be /usr/local/ and it won't need to include -L/usr/local/lib in the link line. |
Yes.
I agree, that's strange. I don't think it should be there (
I hid the and the errors I get from running the tests are the same: |
|
Here's another idea. Not sure if it will work: |
|
You can also try installing GalSim with the system g++: (And don't use the above EXTRA_FLAGS in this case.) |
|
The first one with the The second suggestion should work, but it will probably clash with FFTW and/or TMV, so it'll take some time to re-install these and re-run. I'll get back to you when I'm done. |
|
Hi Mike---short story: compiling GalSim with gcc 4.0.1 works. All my other libraries are compiled with this (Python, Boost, FFTW), except possibly TMV. I'm happy to stay in this configuration. Some side notes, which I know you're aware at least of the second point: (1) To make compilation work with gcc v4.0.1, I had to remove (2) I thought I was having to reset the (3) I had a funny problem with which seemed to cause the problem above. This problem seems to originate from (somehow) scons seeing my other gcc, which is /opt/local/bin/gcc (which gets linked to the preferred gcc version)--if I reset the /opt/local/bin/gcc link (which is done via a |
|
I'd like to mention the problem I had with |
|
Actually, I think your last item about being unable to delete .sconf_temp/conftest_0 is because you did |
|
Hi Mike: Actually, the ordering of command execution was such that I tried This "OSError" message cleared with a following It seems that, although |
Yes. This should probably be in the FAQ. It's especially relevant when you change significant things about your system outside of the GalSim directory -- install new libraries, remove libraries, change symlinks, etc. |
|
Reiko, is everything working for you now here? I added some text to the Install FAQ about this if you want to check it out and make sure I covered everything. Then if all's well, I think we can go ahead and merge this. |
|
Yes Mike, it's all working. I see the Installation FAQ entry---thanks! Please go ahead and merge. |
|
I second the merge (and thanks for the FAQ updates Reiko!) On Thu, Oct 4, 2012 at 7:22 AM, Reiko Nakajima [email protected]:
Barnaby Rowe Department of Physics & Astronomy |
#288 Expose c++ integrator to python and remove scipy dependence
This turned out to be a lot easier than I thought, so this is a pretty short pull request.
The upshot is that the following now works:
I also updated the Da in Cosmology to use this rather than the scipy.quad function, which removes our only scipy dependence.