Skip to content
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

StructureMatcher.get_rms_dist illegal instruction on release pymatgen==2020.6.8. #1868

Closed
mortengjerding opened this issue Jun 10, 2020 · 20 comments

Comments

@mortengjerding
Copy link

Describe the bug
On the most recent release of pymatgen (2020.6.8) I am now getting "Illegal instruction" when running the pymatgen.structure_matcher.StructureMatcher.get_rms_dist. This did not happen on the previous release (2020.4.29).

To Reproduce
The following script

from ase.build import bulk
from pymatgen.io.ase import AseAtomsAdaptor
from pymatgen.analysis.structure_matcher import StructureMatcher
al = bulk('Al')
adaptor = AseAtomsAdaptor()
structure = adaptor.get_structure(al)
StructureMatcher().get_rms_dist(structure, structure)

results in "Illegal instruction (core dumped)".

Expected behavior
The result of the get_rms_dist should be zero.

Desktop (please complete the following information):

morten@hp ~ $ python3
Python 3.6.9 (default, Apr 18 2020, 01:56:04) 
[GCC 8.4.0] on linux
>>> import pymatgen
>>> pymatgen.__version__
'2020.6.8'

@mortengjerding mortengjerding changed the title StructureMatcher.get_rms_dist illegal instruction StructureMatcher.get_rms_dist illegal instruction on release pymatgen==2020.6.8. Jun 10, 2020
@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

If you installed pymatgen via conda, pls try to do pip install --upgrade pymatgen. This error is usually a symptom that something is wrong with the compilation of the C extensions and happens quite frequently on Linux based systems.

@shyuep shyuep closed this as completed Jun 10, 2020
@mortengjerding
Copy link
Author

I have not installed pymatgen through conda. I have simply pip installed it. This is also making my pipelines fail on gitlab so it's not just my laptap. I just ran the above script in a virtual environment and it still fails. I set the venv up like in the following:

$ python3 -m venv venv
$ . venv/bin/activate
$ pip install pymatgen
$ python test_script.py
Illegal instruction (core dumped)

So I don't see that it should be something with my setup. Since you closed the issue I was hoping you could help me figure out what is wrong. As I wrote: It worked in the previous release.

@shyuep shyuep reopened this Jun 10, 2020
@mortengjerding
Copy link
Author

Sorry I forgot that I also had to install ASE in the venv to make the script work. The magic incantation to set up the environment is:

$ python3 -m venv venv
$ . venv/bin/activate
$ pip install pymatgen ase
$ python test_script.py
Illegal instruction (core dumped)

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

Does it fail for all scripts, or is it just the specific code you showed that is an issue? As far as I know, nothing has changed since v2020.4 that would cause this. There were some issues with sympy...

@mortengjerding
Copy link
Author

I'm sorry I don't know whether anything has broken since I am essentially only using the StructureMatcher functionality in my scripts. Could it be that something went wrong in the building of the wheels (I am no expert on this)? Just to make absolute sure that everything works in the previous release I checked this in a venv:

$ python3 -m venv venv
$ . venv/bin/activate
$ pip install pymatgen==2020.4.29 ase
$ python test_script.py
$ # No error

Can the issue be reproduced on your pc?

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

I do not have such an error on my Mac. It would help if you can run other scripts to check if this is something wrong in the core of pymatgen or whether it is just in StructureMatcher. Otherwise, I have no way of debugging this.

@mortengjerding
Copy link
Author

mortengjerding commented Jun 10, 2020

Ok so I have found that the function "find_points_in_spheres" is causing the crash. I have made a minimal example here which crashed on my computer:

import numpy as np                                                                                                                                                                                          
from pymatgen.optimization.neighbors import find_points_in_spheres                                                                                                                                          
                                                                                                                                                                                                            
all_coords = np.array([[0., 0., 0.]], dtype=float)                                                                                                                                                          
center_coords = np.array([[0., 0., 0.]], dtype=float)                                                                                                                                                       
r = 2.863855528636639                                                                                                                                                                                       
pbc = np.array([1, 1, 1])                                                                                                                                                                                   
lattice = np.array([[0., 2.025, 2.025],                                                                                                                                                                     
                    [2.025, 0., 2.025],                                                                                                                                                                     
                    [2.025, 2.025, 0.]], dtype=float)                                                                                                                                                       
                                                                                                                                                                                                            
find_points_in_spheres(all_coords=all_coords,                                                                                                                                                               
                       center_coords=center_coords,                                                                                                                                                         
                       r=r,                                                                                                                                                                                 
                       pbc=pbc,                                                                                                                                                                             
                       lattice=lattice,                                                                                                                                                                     
                       tol=1e-8)

This function is being called when the structurematcher tries to Niggli reduce the input structures. Has something changed in this function?

@mortengjerding
Copy link
Author

I hope this helps, otherwise let me know.

@greschd
Copy link

greschd commented Jun 10, 2020

We're also encountering this issue in our CI, see the aiidateam/aiida-core#4111

My suspicion is that this is caused by the wheel being built against a CPU instruction set that is not fully supported on the machines where it crashes.

@mortengjerding what's the output of cat /proc/cpuinfo for you?

Looking at the wheel itself (pymatgen-2020.6.8-cp38-cp38-manylinux2010_x86_64.whl in our case) it seems it's using AVX2 instructions. Grepping from the root of the unpacked wheel:

$ grep -inR AVX2 .
Binary file ./pymatgen/optimization/neighbors.cpython-38-x86_64-linux-gnu.so matches

If that's the case, the immediate workaround would be installing it without the wheel, i.e.

pip install --no-binary pymatgen pymatgen

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

Ah thank you. Indeed, that is likely the issue. The strange thing is, I never upload wheels for Linux precisely because of this reason. @mkhorton Do you know what happened with the previous release? Usually I just upload the Windows and Mac wheels and for Linux, I just assume that it can be compiled from the tar.gz easily. Linux is very fussy about compilers etc.

@shyuep shyuep closed this as completed Jun 10, 2020
@shyuep shyuep reopened this Jun 10, 2020
@mortengjerding
Copy link
Author

@greschd That would also be my understanding. The output of cpuinfo indicates that my processor supports avx2 instructions:

$ cat /proc/cpuinfo
processor       : 0                                                                                                                                                    
vendor_id       : GenuineIntel                                                                                                                                         
cpu family      : 6                                                                                                                                                    
model           : 142                                                                                                                                                  
model name      : Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz                                                                                                             
stepping        : 10                                                                                                                                                   
microcode       : 0xca                                                                                                                                                 
cpu MHz         : 761.181                                                                                                                                              
cache size      : 6144 KB                                                                                                                                              
physical id     : 0                                                                                                                                                    
siblings        : 8                                                                                                                                                    
core id         : 0                                                                                                                                                    
cpu cores       : 4                                                                                                                                                    
apicid          : 0                                                                                                                                                    
initial apicid  : 0                                                                                                                                                    
fpu             : yes                                                                                                                                                  
fpu_exception   : yes                                                                                                                                                  
cpuid level     : 22                                                                                                                                                   
wp              : yes                                                                                                                                                  
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm\
 constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 sdbg\
 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb invpcid_single pti s\
sbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid mpx rdseed adx smap clflushopt intel_pt xsaveopt xsavec\
 xgetbv1 xsaves dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp md_clear flush_l1d                                                                       
bugs            : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf mds swapgs itlb_multihit                                                                   
bogomips        : 3600.00                                                                                                                                              
clflush size    : 64                                                                                                                                                   
cache_alignment : 64                                                                                                                                                   
address sizes   : 39 bits physical, 48 bits virtual                                                                                                                    
power management:

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

@mortengjerding Can you try pip install --no-binary pymatgen and see if that causes the problem to disappear? We will remove the Linux wheels in future releases.

@greschd
Copy link

greschd commented Jun 10, 2020

Yeah, it could well be a different incompatibility in which instructions are supported..

@mortengjerding Can you try pip install --no-binary pymatgen and see if that causes the problem to disappear? We will remove the Linux wheels in future releases.

Note that you have to pass pymatgen twice to pip - once to tell it which package to ignore binaries for, and once to tell it which package to install.

@mortengjerding
Copy link
Author

mortengjerding commented Jun 10, 2020

I have just tried and can confirm that installing with pip install --no-binary pymatgen pymatgen makes the problem disappear. Thanks a lot for the help!

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

Thanks for the debug. We will handle the rest from our end.

@greschd
Copy link

greschd commented Jun 10, 2020

@shyuep maybe you could remove the Linux wheels for the current release? Not sure what your policy on removing already-published wheels is, but in this case it's likely to fix more issues than it causes.

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

I already did. Removing wheels is fine. For Linux users, they will default to the tar.gz anyway.

@greschd
Copy link

greschd commented Jun 10, 2020

Fantastic, thanks!

@mkhorton
Copy link
Member

Do you know what happened with the previous release?

Yes, this was on me -- I was trying to improve our cross-platform support with the latest release for pip users. Wheels should have been compliant with manylinux2010 (see PEP 571) and seemed to work. However, evidently this is not the case, so thanks for the report @mortengjerding and @greschd and my apologies for the inconvenience. Back to the drawing board on this!

@shyuep
Copy link
Member

shyuep commented Jun 10, 2020

Ok. @mkhorton Let's not distribute wheels for Linux in future. The benefit of wheels on Linux platforms is marginal, since there are seldom compilation issues on those platforms. Wheels are useful mainly for Windows and Macs where the compilers may sometimes not be available.

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

No branches or pull requests

4 participants