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

Added draft of vasp_util.py for VASP trajectories and OTF #82

Merged
merged 20 commits into from
Oct 14, 2019

Conversation

kylebystrom
Copy link
Contributor

The new vasp_util.py file can edit VASP POSCAR files with new coordinates, read forces from a static calculation, and read trajectories from an ionic relaxation/MD calculation. This is a draft; I still need to write unit tests and documentation.

@codecov-io
Copy link

codecov-io commented Oct 1, 2019

Codecov Report

Merging #82 into master will increase coverage by 0.76%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
+ Coverage   50.97%   51.74%   +0.76%     
==========================================
  Files          29       31       +2     
  Lines        4716     4791      +75     
==========================================
+ Hits         2404     2479      +75     
  Misses       2312     2312
Impacted Files Coverage Δ
flare/struc.py 97.19% <100%> (+0.02%) ⬆️
flare/dft_interface/vasp_util.py 100% <100%> (ø)
flare/flare_io.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbcd65b...efe74e2. Read the comment docs.

@stevetorr
Copy link
Contributor

Will throw in some more comments later, but looks good so far!

@kylebystrom kylebystrom changed the title WIP added draft of vasp_util.py for VASP trajectories and OTF Added draft of vasp_util.py for VASP trajectories and OTF Oct 12, 2019
@kylebystrom
Copy link
Contributor Author

This should be ready to go now.

tests/test_flare_io.py Outdated Show resolved Hide resolved
Parse the DFT input in a directory.
:param vasp_input: directory of vasp input
"""
return Structure.from_pmg_structure(Poscar.from_file(poscar).structure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you ensure that this returns the cartesian coordinates every time since that's the FLARE convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed by the unit test for the from_pmg_structure function, but I also will address it in the test_structure_parsing function when I assert the correct coordinate parsing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, test_struc.test_from_pmg_structure doesn't guarantee the cartesian coordinates are read because it sets the unit cell to np.eye(3). So I copy-pasted the test and changed the lattice parameters so that it will fail if from_pmg_structure returns fractional coordinates.

@stevetorr
Copy link
Contributor

This looks great, Kyle! Can you address the two points above: 1. a small addition to the unit test, and 2. A guarantee that wherever in the code a FLARE structure is produced that the cartesian coordinates are returned?
Once that's set, feel free to pull :)

@stevetorr
Copy link
Contributor

Closes #87

@kylebystrom
Copy link
Contributor Author

Okay, this is good to go now. Merging with master.

@kylebystrom kylebystrom merged commit 0d55cab into master Oct 14, 2019
@kylebystrom kylebystrom deleted the kyle/pymatgen_utils branch October 14, 2019 17:29
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

Successfully merging this pull request may close these issues.

3 participants