Add updated compiled MATLAB messenger mex #178
Conversation
|
Was this new build system tested on Windows? It looks like it undoes some On Mon, Apr 6, 2015 at 1:47 PM, Ariel Rokem notifications@github.com
|
|
I don't think it was. I see that you've already started making comments on On Mon, Apr 6, 2015 at 2:02 PM, Ali Ebrahim notifications@github.com
|
|
Gotcha. @jjangsangy: what do you think? |
|
Sure let me take a closer look. Sorry for the late response, I just got back from a hackathon, so I've been a little preoccupied. @aebrahim would love to coordinate with you about those changes so I can implement the code to get running. |
|
OK, I have a new proposed solution. I've made a few more changes, and my testing indicates that everything in this PR now works on Windows, Mac, and Linux. Can I suggest that this be merged now, and then the code be re-factored to the new structure while leaving the logic here relatively intact? Also, the updated make script has a which_matlab function to detect where matlab is when it's not in the system path. Could this function potentially be moved to pymatbridge itself as part of your refactor to autodetect the executable when it can't be found? |
The script fixes bugs so that the script will run on Windows, Linux, and Mac OS X. It can also now automatically detects MATLAB on these platforms if it is not specified. The mexw32 folder was removed as it was unused (contained paramters from Mac)
By using the asynchronous API while listening constantly, excessive CPU usage occurs. Instead, this commit uses the synchronous version of zmq_recv, and uses zmq_poll with a timeout of 20ms to ensure the graphics are updated. This fixes #175.
These have been statically linked to libzmq 4.0.5, so libzmq does not actually need to be installed at the system level. They incorporate the changes made to messenger.c to reduce CPU load.
|
Just chiming in real quick, the |
|
@aebrahim - that sounds like a good way forward. Let me take a look at everything first. First - @blink1073's proposal about detecting the matlab path: are you proposing to grab that module (actually just the top two functions, I think) and put it somewhere in our code (together with the pexpect license)? Surely you're not proposing to make pexpect a dependency? |
|
Yes, I'd say grab the two functions and the license. |
|
+1 for reimplementing Nice bit of trivia. |
| @@ -67,9 +76,55 @@ def build_octave(): | |||
| do_build(make_cmd, 'messenger.mex') | |||
|
|
|||
|
|
|||
| def which_matlab(): | |||
There was a problem hiding this comment.
This will all be replaced by the pexpect stuff that @blink1073 pointed out. Does that work well on Windows?
There was a problem hiding this comment.
Does pexpext work if it's not in the path?
In my which function I look in the default installation location for MATLAB
even if it's not in the path. Annoying because MATLAB uses a different
structure on every platform. It would be great if pexpect could replace
this.
On Apr 10, 2015 4:09 PM, "Ariel Rokem" notifications@github.com wrote:
In messenger/make.py
#178 (comment)
:@@ -67,9 +76,55 @@ def build_octave():
do_build(make_cmd, 'messenger.mex')+def which_matlab():
This will all be replaced by the pexpect stuff that @blink1073
https://github.com/blink1073. Does that work well on Windows?—
Reply to this email directly or view it on GitHub
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28187251.
There was a problem hiding this comment.
Does pexpext work if it's not in the path?
pexept assumes the binaries are in the $PATH since it replicates BSD utils which.
However, get_matlab_bin() should return absolute path of the matlab binary regardless of if it is in the path or not.
Then running get_matlab_env(matlab='matlab') with the path to the binary will also return a dictionary containing that particular configuration environment variables.
The only known instance of where this breaks is if a user has two different installations of matlab installed. If that is the cas, you can override the discovery machinery by specifying the MATLAB_BIN variable in the config.ini file in the root directory to specify the copy of matlab you'd like to target.
There was a problem hiding this comment.
Right.. the function which_matlab which I wrote doesn't reimplement which
at all. It calls "which matlab." However it usually isn't in the path, so
when that fails it looks in the default location on all 3 platforms.
I was actually inspired by your function to make it work on all 3
platforms. However, your code assumes the same structure of MATLAB on all 3
platforms, which for some reason isn't the case (why MATHWORKS?), which is
why I had to use if statements and handle each OS separately.
On Fri, Apr 10, 2015 at 4:41 PM, Sang Han notifications@github.com wrote:
In messenger/make.py
#178 (comment)
:@@ -67,9 +76,55 @@ def build_octave():
do_build(make_cmd, 'messenger.mex')+def which_matlab():
pexept assumes the binaries are in the path since it replicates BSD utils
which.However, get_matlab_bin should return absolute path of the matlab binary
regardless of if it is in the path or not.https://github.com/jjangsangy/python-matlab-bridge/blob/master/messenger/make.py#L206
Then running get_matlab_env with the path to the binary will also return
a dictionary containing that particular configuration environment variables.https://github.com/jjangsangy/python-matlab-bridge/blob/master/messenger/settings.py#L15
—
Reply to this email directly or view it on GitHub
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28188377.
There was a problem hiding this comment.
So the structure can be different.
I know that sometimes there is a R2014_* directory that is present in windows but not on OSX or Linux. I took care of that in this section.
for p in os.listdir(os.path.join(programs[host], *matlab)):
# Checks for this folder in the matlab directory
if p.startswith('R20'):
matlab.append(p)
matlab.append('bin')
Sorry, maybe I should have written more comments. Honestly didn't even think anyone would even read this :)
There was a problem hiding this comment.
Honestly, it was easier for me to rewrite this than try to debug it when it
didn't work on Windows when I tried to merge your PR.
Is it possible to just apply the simplifications you need (like 1 config
file, integration with setup.py) using the functions in my PR?
On Fri, Apr 10, 2015 at 5:00 PM, Sang Han notifications@github.com wrote:
In messenger/make.py
#178 (comment)
:@@ -67,9 +76,55 @@ def build_octave():
do_build(make_cmd, 'messenger.mex')+def which_matlab():
So the structure can be different.
I know that sometimes there is a R2014_* directory that is present in
windows but not on OSX or Linux. I took care of that with this code.Sorry, maybe I should have written more comments. Honestly didn't even
think anyone would even read this :)for p in os.listdir(os.path.join(programs[host], *matlab)): if p.startswith('R20'): matlab.append(p) matlab.append('bin')—
Reply to this email directly or view it on GitHub
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28189008.
There was a problem hiding this comment.
Also dude, it wasn't bad to read; it's just a bit different than my coding
style so.
On Fri, Apr 10, 2015 at 5:05 PM, Ali Ebrahim aebrahim@ucsd.edu wrote:
Honestly, it was easier for me to rewrite this than try to debug it when
it didn't work on Windows when I tried to merge your PR.Is it possible to just apply the simplifications you need (like 1 config
file, integration with setup.py) using the functions in my PR?On Fri, Apr 10, 2015 at 5:00 PM, Sang Han notifications@github.com
wrote:In messenger/make.py
#178 (comment)
:@@ -67,9 +76,55 @@ def build_octave():
do_build(make_cmd, 'messenger.mex')+def which_matlab():
So the structure can be different.
I know that sometimes there is a R2014_* directory that is present in
windows but not on OSX or Linux. I took care of that with this code.Sorry, maybe I should have written more comments. Honestly didn't even
think anyone would even read this :)for p in os.listdir(os.path.join(programs[host], *matlab)): if p.startswith('R20'): matlab.append(p) matlab.append('bin')—
Reply to this email directly or view it on GitHub
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28189008
.
There was a problem hiding this comment.
I suggest that we finish this PR, and then refine the config system on
another PR.
On Fri, Apr 10, 2015 at 5:06 PM, Ali Ebrahim notifications@github.com
wrote:
In messenger/make.py
#178 (comment)
:@@ -67,9 +76,55 @@ def build_octave():
do_build(make_cmd, 'messenger.mex')+def which_matlab():
Honestly, it was easier for me to rewrite this than try to debug it when
it didn't work on Windows when I tried to merge your PR. Is it possible to
just apply the simplifications you need (like 1 config file, integration
with setup.py) using the functions in my PR?
… <#14ca5cc41acde2db_>
On Fri, Apr 10, 2015 at 5:00 PM, Sang Han notifications@github.com
wrote: In messenger/make.py <
https://github.com/arokem/python-matlab-bridge/pull/178#discussion_r28189008>
: > @@ -67,9 +76,55 @@ def build_octave(): > do_build(make_cmd,
'messenger.mex') > > > +def which_matlab(): So the structure can be
different. I know that sometimes there is a R2014_* directory that is
present in windows but not on OSX or Linux. I took care of that with this
code. Sorry, maybe I should have written more comments. Honestly didn't
even think anyone would even read this :) for p in
os.listdir(os.path.join(programs[host], *matlab)): if p.startswith('R20'):
matlab.append(p) matlab.append('bin') — Reply to this email directly or
view it on GitHub <
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28189008>.—
Reply to this email directly or view it on GitHub
https://github.com/arokem/python-matlab-bridge/pull/178/files#r28189149.
There was a problem hiding this comment.
Could you embed the call to the pexpect which in a function that will gracefully handle failure modes (e.g. matlab not on the path)?
|
So I've gone ahead and applied the code from I think this should resolve the issues we were having with windows not building. If I have some time later today, I will try to install Windows and Matlab on a VM and run a sanity check. |
|
Oops. Didn't test on py3. My bad. I will make the required changes. It's also worth noting is that most users shouldn't need to compile MATLAB
|
|
That's good, I'm getting some rather scary-looking matlab crash with this new mex file: |
|
Yikes. Ok maybe I shouldn't compile on mac then. Do my Windows and Linux
|
|
This was from a mex I compiled on my mac, but I get the same thing from Maybe it has something to do with my environment OMP settings? I will try On Fri, Apr 10, 2015 at 4:33 PM, Ali Ebrahim notifications@github.com
|
|
I forgot I had made a Windows compliant version of pexpect's which here: |
|
I already handle the not in path failure mode. I'm not sure what exactly On Fri, Apr 10, 2015 at 5:20 PM, Steven Silvester notifications@github.com
|
|
That's fantastic! How do you 'forget' that? :-) On Fri, Apr 10, 2015 at 5:20 PM, Steven Silvester notifications@github.com
|
|
Too many irons in the fire. :) — On Fri, Apr 10, 2015 at 7:22 PM, Ariel Rokem notifications@github.com
|
|
I can give it a try. Expect a PR against your branch. On Fri, Apr 10, 2015 at 5:22 PM, Ali Ebrahim notifications@github.com
|
|
With the caveat that I can't really test it, because I get that nasty crash On Fri, Apr 10, 2015 at 5:23 PM, Ariel Rokem arokem@gmail.com wrote:
|
|
Also, thanks guys for all your work on this module. I really love it for On Fri, Apr 10, 2015 at 5:21 PM, Ali Ebrahim aebrahim@ucsd.edu wrote:
|
|
I'm surprised you get that crash even when you compile yourself. Is it
|
|
OK - I think this is the pexpect 'which' implementation: https://github.com/aebrahim/python-matlab-bridge/pull/1. Is it overkill? |
|
And I still get that nasty dump when compiling this. I'll keep trying a bit more on that. |
|
Also: happy to hear that you are having a good time hacking on this, and that it's been useful for you. I am also enjoying this! |
|
@arokem @aebrahim are either of you running a scipy stack compiled using intel MKL, Anaconda MKL or Intel Parallel Studio C++ compositor? The issue may be related to this problem with matlab compiling against wrong BLAS library at link time. https://software.intel.com/en-us/forums/topic/486842 |
|
No my mac install is super vanilla.
|
|
I used clang
|
|
Interesting, that's similar to my configuration. Actually went and cloned this fork, compiled messenger and installed a fresh copy and everything seems to be working fine. I kind of cheated and just ran the mex build command manually on the terminal rather than doing it through |
|
Bingo! That must be it. I switched to mkl last week On Friday, April 10, 2015, Sang Han notifications@github.com wrote:
|
|
OK - the problem I've seen in the tests is also intermittent. That is, I Is this ready to be merged from your point of view @aebrahim? On Fri, Apr 10, 2015 at 9:40 PM, Ariel Rokem arokem@gmail.com wrote:
|
|
Yup compete on my end
|
Add updated compiled MATLAB messenger mex
|
For future reference, my MKL pains (see above) have been resolved following advice from here: http://stackoverflow.com/questions/20554074/sklearn-omp-error-15-when-fitting-models And setting: In my bash configuration file |
The source files were changed in #176.
These have been statically linked to libzmq 4.0.5, so libzmq does not
actually need to be installed at the system level.