Added methods to compute and compare DOS fingerprints#2772
Added methods to compute and compare DOS fingerprints#2772janosh merged 35 commits intomaterialsproject:masterfrom
Conversation
…into dos_fingerprinting
for more information, see https://pre-commit.ci
…into dos_fingerprinting git pull
pymatgen/electronic_structure/dos.py
Outdated
| "some error in the spelling." | ||
| ) | ||
|
|
||
| def _fp_to_dict(self, fp): |
There was a problem hiding this comment.
Yes, It could be. Will make the changes. Thanks for the feedback 😃
JaGeo
left a comment
There was a problem hiding this comment.
You could replace one "finger print" with fingerprint in the documentation but otherwise it looks fine from my side.
Thanks for the review @JaGeo 😄 |
|
Hi @janosh , This PR is ready to be merge. |
janosh
left a comment
There was a problem hiding this comment.
Thanks @naik-aakash! I left some comments.
pymatgen/electronic_structure/dos.py
Outdated
| upper_band_edge = energies[np.argmax(densities)] | ||
| return upper_band_edge | ||
|
|
||
| def get_dos_fp(self, type="summed_pdos", binning=True, min_e=None, max_e=None, nbins=256, normalize=True): |
There was a problem hiding this comment.
Would be good to add type annotations here.
def get_dos_fp(self, type: str = "summed_pdos", binning: bool = True, ...There was a problem hiding this comment.
Thanks for the suggestions , I have made the requested changes.
pymatgen/electronic_structure/dos.py
Outdated
| type (str): Specify fingerprint type needed can accept 's/p/d/f/summed_pdos/tdos' | ||
| min_e (float): The minimum mode energy to include in the fingerprint | ||
| max_e (float): The maximum mode energy to include in the fingerprint | ||
| nbins (int): Number of bins to be used in the fingerprint |
There was a problem hiding this comment.
Can we rename this to n_bins for proper snake casing?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Remove unnecessary file
|
Hi @janosh , all suggestions have been incorporated now. And tests seems to pass as well. 😄 |
janosh
left a comment
There was a problem hiding this comment.
@naik-aakash Thanks, looks great! Just a few more nitpicks.
|
Hi @janosh , thank you for the really useful tips and suggestions. 😃 |
|
Hi @janosh , I do not know why so many tests are failing. I looked at Failed tests and they do not have anything to do with this PR. I hope it could still be merged. |
|
@naik-aakash Sorry for the delay here. Back from traveling now. PR looks good! Made a few last changes. Failures like you said are unrelated. Ready to merge. |
@janosh No worries! Thank you. Happy Holidays |
…ct#2772) * added dos_fingerprint and similarity index methods * Added test cases and reformatted and cleaned code * added binwidth ,renamed states to densities in fp obj,updated tests * added source link * changed get_dos_fp_similarity and fp_to_dict methods to static * Delete Test.py, unnecessary file * simplified dict updating, added missing type annotations * NamedTuple return type fixed * small clean up * document get_dos_fp() and get_dos_fp_similarity() raise conditions in doc str * add types for fp1,fp2 and update doc str * update exception tests Co-authored-by: anaik <anaik@sv2218.zit.bam.de> Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
…ct#2772) * added dos_fingerprint and similarity index methods * Added test cases and reformatted and cleaned code * added binwidth ,renamed states to densities in fp obj,updated tests * added source link * changed get_dos_fp_similarity and fp_to_dict methods to static * Delete Test.py, unnecessary file * simplified dict updating, added missing type annotations * NamedTuple return type fixed * small clean up * document get_dos_fp() and get_dos_fp_similarity() raise conditions in doc str * add types for fp1,fp2 and update doc str * update exception tests Co-authored-by: anaik <anaik@sv2218.zit.bam.de> Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
…ct#2772) * added dos_fingerprint and similarity index methods * Added test cases and reformatted and cleaned code * added binwidth ,renamed states to densities in fp obj,updated tests * added source link * changed get_dos_fp_similarity and fp_to_dict methods to static * Delete Test.py, unnecessary file * simplified dict updating, added missing type annotations * NamedTuple return type fixed * small clean up * document get_dos_fp() and get_dos_fp_similarity() raise conditions in doc str * add types for fp1,fp2 and update doc str * update exception tests Co-authored-by: anaik <anaik@sv2218.zit.bam.de> Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
* added dos_fingerprint and similarity index methods * Added test cases and reformatted and cleaned code * added binwidth ,renamed states to densities in fp obj,updated tests * added source link * changed get_dos_fp_similarity and fp_to_dict methods to static * Delete Test.py, unnecessary file * simplified dict updating, added missing type annotations * NamedTuple return type fixed * small clean up * document get_dos_fp() and get_dos_fp_similarity() raise conditions in doc str * add types for fp1,fp2 and update doc str * update exception tests Co-authored-by: anaik <anaik@sv2218.zit.bam.de> Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
Compute and compare fingerprints of project and total density of states.
This functionality could be really useful to compare the DOS of materials from different softwares (eg:- LOBSTER and VASP ) or versions (Eg:- VASP v5 and VASP v6)