pace-neutrons / pace-python Goto Github PK

For complex operations, pace_python almost always segfaults on exit on Linux.

Run the following script:

import pace_python
m = pace_python.Matlab()
sw = m.spinw()
print(sw)

This produces a segfault. If the final line print(sw) is removed, there is no segfault... there's something screwy with the garbage collector...

The following base matlab-python code also segfaults:

import pace_python.pace
m = pace_python.pace.initialize()
sw = m.call('spinw', [], nargout=1)
m.call('disp', [sw], nargout=1)
m.exit()

(the segfault occurs irregardless of whether the final m.exit() is there or not). However:

import pace_python.pace
m = pace_python.pace.initialize()
sw = m.call('spinw', [], nargout=1)
sws = m.call('struct', [sw], nargout=1)
print(sws)

Does not segfault - suggesting it's something about the disp call...?

Issue

The data encoder / decoder is failing for 2D non-float numpy arrays.

How to reproduce

Run this

import numpy as np
from pace_neutrons import Matlab
m = Matlab()

bragg_peaks = np.array([[4,0,0], [2,0,0], [1,1,0], [4,4,0], [1,0,1]]).T
m.svd(bragg_peaks)

which will fail with a ValueError: initializer must be a rectangular nested sequence in line 47 of DataTypes.py.

If the values in the array are doubles (e.g. bragg_peaks = np.array([[4.,0.,0.], [2.,0.,0.], [1.,1.,0.], [4.,4.,0.], [1.,0.,1.]]).T) then the script will work.

System details (version, OS)

v0.2.0 on IDAaaS (SL7)

Because we're using "in-process" loading (where Matlab is loaded into the Python process like another module), libraries needed by Matlab are also loaded into the same process. Matlab ships with its own version of the BLAS/LAPACK libraries (Intel MKL), and there appears to be a name collision with the BLAS/LAPACK libraries used by numpy / scipy. Specifically calls from numpy / scipy seem to reach the Matlab libraries rather than the libraries numpy / scipy was originally linked to on a particular system. This is because in order to get pace_python to work, we had to load Matlab first, so its libraries get loaded before others. This probably means the numpy / scipy BLAS/LAPACK libraries never get loaded.

This is fine if the numpy / scipy on a particular environment was linked to the MKL, but not if it was linked to another library. The specific errors seen (e.g. Intel MKL ERROR: Parameter 5 was incorrect on entry to ZHEEVD.) seems to indicate that the problem is due to Fortran vs C calling conventions for some parameters. numpy / scipy exposes the Fortran interface in _flapack which expects all parameters to be passed by reference; the Matlab MKL seems to expects scalar parameters (such as the leading dimension lda) to be passed by value.

So far systems with numpy / scipy packages which work are:

• conda using conda-forge packages.
• Ubuntu 18.04 provided packages.
• Windows pip packages.

System which doesn't

• PyPi / pip provided numpy / scipy (manylinux) packages.

These are UI bugs when using pace-neutrons with Spyder.

• When typing commands, Spyder uses the getdoc method to get instant documentation... but we don't provide this, so get errors like Unrecognized function or variable 'saveNXSPE.getdoc'. Edit: the cause of this error is in IPython - the getdoc method doesn't exist and Matlab returns an error which is caught and ignored by Python but Matlab also prints out a message which is not ignored, due to some stuff in the Matlab-Python bridge / stdout/err redirection.
• Some commands don't work when run in a cell (using the Spyder runcell because of stdout/stderr redirection causes Matlab to give a Error on flush (see here)
• After some processing, Spyder does something which causes the Matlab runtime library to lose its initialization. This manifests as an ImportError on matlab_pysdk.runtime.NumberOfOutputArgumentsError because the *Error classes are actually in the MCR C++ extension and are patched into the module at runtime (see here). Edit: the cause of this issue is that Spyder deletes the modules here [a normal reload is not an issue].

gen_sqw is not working on pace-python.

So there are two related problems to this.

1. The original script the user used to report this bug uses things like u = np.array([1,0,0]) which creates a np.int64 array and is converted into a Matlab int array which does not work with most Matlab methods which expect double. The solution is to do a (possibly expensive) conversion to double.
2. Cluster configuration - on the DAaaS system where this is run, the default configuration is to run with a Herbert cluster but this doesn't work. Unfortunately, as explained in Herbert #418 the cluster cannot be turned off...

Issue

When running a "cell" in Spyder, the output redirection apparently doesn't work, giving an

Error writing to output stream.
: iostream error

How to reproduce

On IDAaaS, start pace_neutrons under Spyder with:

/mnt/nomachine/isis_direct_soft/pace_neutrons -s

Create a script with the following:

from pace_neutrons import Matlab
m = Matlab()

#%%

hpc_status = m.hpc()

The #%% is a cell delimiter. The hpc_status = m.hpc() is in the second cell (index 1). Now run the cell with: (in the console):

runcell(1, '/path/to/file.py')

(or press Ctrl+Enter when the hpc() line is selected).

The error should appear. If you instead run the full file with:

runfile('/path/to/file.py')

(or click the green play triangle button) then everything works ok.

System details (version, OS)

v0.2.0 on IDAaaS (SL7)

If I call pace_neutrons I get the following error:

Traceback (most recent call last):
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\Scripts\pace_neutrons.exe\__main__.py", line 7, in <module>
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\site-packages\pace_neutrons_cli\cli.py", line 88, in main
    _set_env(mlPath, force_reload)
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\site-packages\pace_neutrons_cli\cli.py", line 33, in _set_env
    if DET.env_not_set() or force_reload:
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\site-packages\pace_neutrons_cli\utils.py", line 231, in env_not_set
    if self.path_var not in os.environ:
  File "C:\Users\MLL13652\Miniconda3\envs\py37_pace\lib\site-packages\pace_neutrons_cli\utils.py", line 184, in find_version
    sub_dirs = next(os.walk(root_dir))[1]
StopIteration

This arises because env_not_set gets a list of directories from the PATH environment variable, and then walks through their subdirectories in find_version without checking that the PATH directories exist first, causing StopIteration immediately.

I guess this won't affect most people, but could easily arise if directory names change, or a program isn't uninstalled correctly.

MatlabProxyObject has doubled properties and methods! (culprit could be __dir__()) - sort it out!!! - Use list(set(...))?

Instead of print to get inline images use getframe and imwrite...?

Better handling of numeric type conversions: kk = kk.set_fun(disp2sqwfun, [euobj.horace_disp, [scalefac], intrinsic_fwhm]); where scalefac is integer in Python

The Jenkins setup needs to be modified to support MacOS.

This is mainly an issue for libpymcr - at the moment for Linux, libpymcr just searches "commonly used" locations for Matlab such as /usr/local and /opt. If the user (or CI system) installs Matlab or the MCR in a different location, libpymcr will not be able to find it unless the user tells it where to look.

On Windows, it is possible to search the registry for all installation Matlab locations. There are some Linux equivalent to this (such as locate) but this might take a bit of time to search and adds to the initialization time. At the very least though we should be searching the full system PATH which should not take very long.

Multiple return values in Python functions (e.g. tuple to cell conversion in call_python) appears to be not working, e.g.

import numpy as np

# Define dispersion relation
def fe_disp(h, k, l, p):
    js = p[0]
    d = p[1]
    om = d + (8*js) * (1 - np.cos(np.pi * h) * np.cos(np.pi * k) * np.cos(np.pi * l))
    return (om, om*0+1)

# Parameters for dispersion: J1=35meV, D=0
pars = [35, 0]

# Plot dispersion
Qlist = np.array([[3/4, 1/4, 1], [1/2, 1/2, 1], [1/2, 0, 1], 
                  [3/4, 1/4, 1], [1, 0, 1], [1/2, 0, 1]])
Qlab  = ['P', 'M', 'X', 'P', '\Gamma', 'X']
energy_bins = [0, 1, 1000]
fwhm = 30
w = m.disp2sqw_plot(Qlist, fe_disp, pars, energy_bins, fwhm, 'labels', Qlab)

gives a MatlabRuntimeError:

Error using disp2sqw_plot (line 145)
The provided dispersion function does not appear to return spectral weight

Some operator overloading doesn't work in the Python wrapper - e.g. w2*3 or w1+w2 where the objects are of sqw or dnd classes - these should call the Matlab mtimes or plus methods but currently do not.

Operator overloading should also support lists / arrays of sqw and dnd objects...

When I try and install from PyPI:

python -mpip install pace_neutrons
pace_neutrons

I get the error:

This is the first time PACE has been run, and we could not find a suitable Matlab Compiler Runtime (MCR) installed.
Do you want to automatically install the MCR? ("y" or "n")y
Traceback (most recent call last):
  File "C:\Users\Miniconda3\envs\py37\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Users\Miniconda3\envs\py37\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Users\Miniconda3\envs\py37\Scripts\pace_neutrons.exe\__main__.py", line 7, in <module>
  File "C:\Users\Miniconda3\envs\py37\lib\site-packages\pace_neutrons_cli\cli.py", line 87, in main
    _set_env(mlPath, force_reload)
  File "C:\Users\Miniconda3\envs\py37\lib\site-packages\pace_neutrons_cli\cli.py", line 40, in _set_env
    mlPath = install_MCR(interactive=True)
  File "C:\Users\Miniconda3\envs\py37\lib\site-packages\pace_neutrons_cli\utils.py", line 352, in install_MCR
    with open(lic_file, 'r') as lic:
FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\Miniconda3\\envs\\py37\\lib\\site-packages\\pace_neutrons_cli\\MCR_license.txt'

The MCR_license.txt is not in either the pace_neutrons or pace_neutrons_cli directories

Matlab introduce +packages and namespaces accessed using the dot notation, e.g. for SpinW, swplot.zoom(). However, these cannot be accessed using the Python Matlab class which assumes that all attributes are functions.

E.g. m.swplot.zoom() fails because it tries to call a Matlab swplot function which does not exist.

If I pass a float kwarg to a simulation function via disp2sqw_eval, it ends up as a nested list. e.g.

>>> cut_sim = m.disp2sqw_eval(m.d1d(qcut), euobj.horace_disp, ('frequency_scale', 1.0, 'intensity_scale', 1.0), fwhh, 'all')
intensity_scale [[1.]]
frequency_scale [[1.]]

The same happens if the arguments are not explicitly named:

>>> cut_sim = m.disp2sqw_eval(m.d1d(qcut), euobj.horace_disp, (1.0, 1.0), fwhh, 'all')
intensity_scale [[1.]]
frequency_scale [[1.]]

Or if only the first argument is set via a scalar

>>> cut_sim = m.disp2sqw_eval(m.d1d(qcut), euobj.horace_disp, 1.0, fwhh, 'all')
intensity_scale [[1.]]
frequency_scale 1.0

This is on Windows with release pace-python 0.1.1, but with my own branch version of euphonic_sqw_models.

When changing the backend in Spyder (Tools->Preferences->IPython Console->Graphics->Backend) from either Automatic or Qt5 to Inline when a Matlab figure is open, a hard crash occurs.

Is not reproducible in Ubuntu 20.04 with Spyder 5.2.2 and pace-neutrons 1.4

On Linux systems, users must set the LD_LIBRARY_PATH environment variable before starting Python.

At present, the initialisation routines tries to detect where Matlab / MCR is installed and sets this programmatically but this does not seem to be respected by the mex files and it fails to load its required libraries.

If symlinks to the required libraries (libmex.so and libMatlabDataArray.so) are provided then a segfault occurs due to incompatibility between the Matlab binaries and the system / Python libstdc++.so.6 and libgcc_s.so.1. (When LD_LIBRARY_PATH is defined it is search first before all other search paths so the Matlab provided libraries are picked up when Python loads).

When running pace_neutrons under Spyder, Matlab error messages seem to pile up. E.g.

m.eig(m.rand(3,4))
m.rand('a')
m.diag()

The error messages for each error is then repeated on the next error.

This seems to be an issue in Spyder itself / how Spyder handles the C-level stdout/stderr which Matlab uses... (e.g. it is present even without our redirection magic, and it is not seen in Jupyter notebooks).

Edit: This does not seem to be an issue in Windows

When running pace_neutrons for the first time, after accepting the prompt to automatically install the MCR, I got an FileNotFoundError for MCR_license.txt.
Manually adding the file resolved the problem.

Issue:

The Brille test (test2_FeBrille()) appears to cause the tests found in test/run_test.py to hang upon completion when running on Linux.

Temporary solution:

Disable test to avoid the hang which would otherwise result in the CI job continuing indefinitely

Context

The current method utilised to produce the .whl file of python setup.py bdist_weel is considered deprecated and is discouraged (https://setuptools.pypa.io/en/latest/userguide/quickstart.html) in favour of using python -m build.

Issue

Currently it is possible to feed cmake options to setup.py and subsequently to the cmake build command by running a command, such as the following where we are specifying the specific version of Matlab to use.

python setup.py bdist_wheel -DMatlab_ROOT_DIR="path/to/matlab/"

This currently works fine, however in the future when we may wish to transition to using python -m build or similar, this functionality will need to be revisited, providing we still require it. I have been unable to work out a solution to this so far with python -m build -DMatlab_ROOT_DIR="path/to/matlab/" not being possible currently.

Some Horace classes were designed as value classes - this includes all the re-written sqw, dnd, multifit and tobyfit classes which are extremely slow in Python because any changes to their properties which does not use explicit getter/setters needs to re-assign the class back, which involves copying a massive amount of data.

As such these classes should be wrapped in Python so they behave more like Python classes (e.g. what Matlab calls "handle" classes).

Issue

Running bragg_positions_view gives an error due to string (from keyboard) parsing in Python/Matlab bridge.

How to reproduce

sqw_file = 'iron.sqw'   # Use Iron dataset from EDATC
bragg_peaks = np.array([[4.,0.,0.], [2.,0.,0.], [1.,1.,0.], [4.,4.,0.], [1.,0.,1.]])
rlu0, width, wcut, wpeak = m.bragg_positions(sqw_file, bragg_peaks, 1.5, 0.06, 0.4,
                                             1.5, 0.06, 0.4, 20, 'gauss', 'bin_ab')
m.bragg_positions_view(wcut, wpeak)

will give an error:

Enter one of the following:
  - peak number (1-5) and scan number (1-3) e.g. 5,3
  - <CR> to continue from present peak and scan (1,1)
  - Q or q to quit
Type option: Error using regexprep
The 'STRING' input must be either a char row vector, a cell array of char row vectors, or a string array.

System details (version, OS)

v0.2.0 on IDAaaS (SL7)