SOC WAVECARs and general cleanup

[mturiansky]

Summary

• add ability to read SOC WAVECARs
• seemingly robust ability to determine if WAVECAR is from std, gam, or ncl binary
• add conversion from WAVECAR to UNK files for wannier90 (will add needed support for UNKs from ncl calculations)

Additional dependencies introduced (if any)

None.

TODO (if any)

• update evaluate_wavefunc and fft_mesh functions to handle ncl WAVECARs
• update get_parchg for ncl WAVECARs
• update old tests
• add new tests for ncl WAVECAR
• add UNK class + tests
• add function to generate UNKs from WAVECAR
• type annotations

Checklist

Before a pull request can be merged, the following items must be checked:

• Code is in the standard Python style.
  Run pycodestyle and flake8
  on your local machine.
• Docstrings have been added in the Google docstring format.
  Run pydocstyle on your code.
• Type annotations are highly encouraged. Run mypy
  to type check your code.
• Tests have been added for any new functionality or bug fixes.
• All existing tests pass.

[mturiansky]

The tests are failing because of sympy (not related to my work so far). Looks like check_assumptions is no longer in sympy.core.assumptions.

[mkhorton]

Thanks @mturiansky. As far as I can see it is still in sympy.core.assumptions, been meaning to take a look at this. Nevertheless not related to this PR, is this ready to merge except for this test?

[mturiansky]

I still want to add the WAVECAR to UNK conversion. I'll mark it "ready for review" to turn it from a draft to a PR when it's ready.

[mkhorton]

Great, thanks.

[mturiansky]

By the way, #1585 can probably be closed now, it was addressed previously but will definitely be addressed now.

Also, heads up @bernstei, I removed the "gamma" argument and replaced it with the "vasp_type" argument. If you don't specify "gamma" it will work fine, but if you specified "gamma" somewhere, then you will get a unexpected keyword TypeError.

[mturiansky]

This PR is ready to merge. The failing circleci is not my fault.

That being said, I think I know how to fix it. I played around with your docker image and was able to fix the sympy error. The version of sympy that is installed by conda seems to be bad. My guess is that it was generated with an older python version and something with the module loading has changed. If you install sympy with pip instead of conda, everything works. However, I found that using "conda uninstall sympy" didn't uninstall things completely/correctly. Here is the sequence of commands I used:

$ docker pull materialsvirtuallab/circle-ci-pmg-py3:3.7.3
$ docker run --name test -it materialsvirtuallab/circle-ci-pmg-py3:3.7.3 bash

and then inside the docker container:

$ export PATH=$HOME/miniconda3/bin:$PATH
$ conda config --system --add channels conda-forge
$ # NOTE: no sympy in next command
$ conda create -q -n test-environment python=3.7 numpy scipy matplotlib cython pandas networkx pytest pip cmake h5py openbabel python-igraph
$ source activate test-environment
$ git clone https://github.com/materialsproject/pymatgen.git && cd pymatgen/
$ pip install --ignore-installed -r requirements.txt -r requirements-optional.txt -r requirements-dev.txt
$ pip install -e .
$ # now, check that it worked with one of the tests that failed before
$ pytest pymatgen/analysis/tests/test_surface_analysis.py

I hope that fixes things, @shyuep @mkhorton . You may need to clear the caches on circleci (is that a thing you can do? I haven't used circleci myself).

[mturiansky]

See #1864

[mturiansky]

Can I get a release once this is merged, please?

[mkhorton]

Sure thing, and thanks for investigating the failed test. CircleCI calculates a hash to determine if it needs to clear its cache, I'll check the logic there.

[mkhorton]

This PR is excellent, thanks for all your work on it.

[mturiansky]

Thanks! I noticed that CirceCI calculates a hash, but it only does this for the requirements*.txt files. When you change the CircleCI config file, which has the conda installation scripts, the cache isn't regenerated, so the changes don't propagate. I addressed this in my other pull request.

[mkhorton]

Release pushed. Trying out a new way of building wheels for PyPI for this release, let me know if there are any issues.

[mturiansky]

Thanks, again! Seems to work well on my machine so far.

[mkhorton]

We are having an issue during building for Windows: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=172210&view=logs&j=171a126d-c574-5c8c-1269-ff3b989e923d&t=1183ba29-a0b5-5324-8463-2a49ace9e213&l=2044

ImportError: cannot import name 'FortranEOFError'

Does this depend on a specific scipy version?

[mturiansky]

Oh boy, I hadn't even thought of that. The fortran reader was added as early as v0.13, but the FortranEORError wasn't added until v1.4.0 as far as I can tell.

[mkhorton]

No worries, I can push a supplemental release if necessary.

[mangerij]

Is there documentation on this? Or an example on how to use it? :)

[mturiansky]

Hi @mangerij , you can read the api documentation for the Wavecar here. There's a few different ways to use the code:

• use the convenience functions like get_parchg or write_unks
• use fft_mesh to get the pseudowavefunctions on a FFT grid
• use the Wavecar.coeffs variable directly, which stores the pseudowavefunction coefficients on the reduced grid in reciprocal space (the grid points are in Wavecar.Gpoints)
  It really depends on what you're trying to do with it.

[mangerij]

Thanks Mark. Everything seems to work pretty nicely. I'm interested in write_unk for the SOC WAVECARs to try to feed to wannier90. However, my WAVECAR is pretty large... 5GB, so it seems to be pretty slow. Any tips to speed up? I am wondering if we could add some gpu Accelerate library lines to some of these classes as an option to speed up these member functions if needed. Might play around with that

…

________________________________ From: Mark Turiansky <notifications@github.com> Sent: Monday, June 22, 2020 10:23 PM To: materialsproject/pymatgen <pymatgen@noreply.github.com> Cc: Mangeri, John <john.mangeri@uconn.edu>; Mention <mention@noreply.github.com> Subject: Re: [materialsproject/pymatgen] SOC WAVECARs and general cleanup (#1861) *Message sent from a system outside of UConn.* Hi @mangerij<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmangerij&data=02%7C01%7Cjohn.mangeri%40uconn.edu%7Cc14e52fefa2447dcdd5808d816ea32b2%7C17f1a87e2a254eaab9df9d439034b080%7C0%7C0%7C637284542420499309&sdata=UZVld7UWFQKsi6ZzJx6KNQcJlGEyAqaHMTQtbP8J%2BSA%3D&reserved=0> , you can read the api documentation for the Wavecar here<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpymatgen.org%2Fpymatgen.io.vasp.outputs.html%23pymatgen.io.vasp.outputs.Wavecar&data=02%7C01%7Cjohn.mangeri%40uconn.edu%7Cc14e52fefa2447dcdd5808d816ea32b2%7C17f1a87e2a254eaab9df9d439034b080%7C0%7C0%7C637284542420509304&sdata=kJQevRycXdbTAtGpU2jvq2M71B7duMsC0xzTWjY18hE%3D&reserved=0>. There's a few different ways to use the code: * use the convenience functions like get_parchg or write_unks * use fft_mesh to get the pseudowavefunctions on a FFT grid * use the Wavecar.coeffs variable directly, which stores the pseudowavefunction coefficients on the reduced grid in reciprocal space (the grid points are in Wavecar.Gpoints) It really depends on what you're trying to do with it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmaterialsproject%2Fpymatgen%2Fpull%2F1861%23issuecomment-647750717&data=02%7C01%7Cjohn.mangeri%40uconn.edu%7Cc14e52fefa2447dcdd5808d816ea32b2%7C17f1a87e2a254eaab9df9d439034b080%7C0%7C0%7C637284542420509304&sdata=a6yx9cFXAti3IUygm6NQqwx4Jip2cyQ9i5hgGLfmrj4%3D&reserved=0>, or unsubscribe<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABZ65FECXNG5JVLLXT2LNKDRX64V7ANCNFSM4NRLWKVA&data=02%7C01%7Cjohn.mangeri%40uconn.edu%7Cc14e52fefa2447dcdd5808d816ea32b2%7C17f1a87e2a254eaab9df9d439034b080%7C0%7C0%7C637284542420519294&sdata=S7VkZtyA9GqQZEQWAzJMiETF1ewdHQE8dtiqR%2FqI%2FvY%3D&reserved=0>.

[mturiansky]

I believe most of the code is IO limited. Definitely in reading in the WAVECAR, you're limited by IO. A few of the computations can be sped up using something like numba or cython in principle (for example, generating the Gpoints), but the speed up you'll gain is essentially negligible for large files like this.

For writing the UNK files, I think you're again mostly IO limited, but generating the FFT mesh could probably be sped up. Right now, it has a python loop that could probably be sped up with numba. If I have some time, I'll do some profiling and see if that would make a noticeable difference.