search

materialsproject / pymatgen-analysis-defects

Defect analysis modules for pymatgen
https://materialsproject.github.io/pymatgen-analysis-defects
Other
39 stars 10 forks source link

Interstitial Generator Have to Take CHGCAR? #59

Closed JiQi535 closed 1 year ago

JiQi535 commented 1 year ago

Hi!

I cannot find the previous method in InterstitialGenerator, which takes just a pmg structure and target element to insert and finds possible interstitial sites. Now, it seems that the CHGCAR is compulsory. May I ask why the previous method is removed?

For reference, the above mentioned legacy InterstitialGenerator is in the last release of pymatgen that still contains the InterstitialGenerator, which is v2022.7.19. And the generator was defined this way: https://github.com/materialsproject/pymatgen/blob/0adae3b3eab8217be6d1abbb314d1b733dfebad6/pymatgen/analysis/defects/generators.py#L149-L187

jmmshn commented 1 year ago

Hi, @JiQi535 the previous version was removed because we found it basically impossible to train our team members to use it and it was not maintained at all. And the old charge insertion class relies on a lot of improper use of the data frames which was slow and difficult to maintain and difficult to build databases with. I originally designed the new package to simply mask the old one but it was decided that the old one should just be removed to avoid package name confusion. https://github.com/materialsproject/pymatgen/pull/2582

In our extensive testing the AECCAR is the most reliable way of identifying interstitials. That is why we added this first.

jmmshn commented 1 year ago

If you want to use the older interstitial finders you can just install the older pymatgen as discussed here in #55

JiQi535 commented 1 year ago

Thanks for the explanation. I personally think the previous code is quite nice to find tetrahedral and octahedra interstitial sites, which uses different method than using CHGCARs. Would it be possible and necessary to have it back somewhere in pymatgen or pymatgen-analysis-defect? Otherwise, I will close the issue.

jmmshn commented 1 year ago

I will take some time and add the Voronoi generator back in but that will have to wait until January since I'm focused on some radiative stuff now. If you need it more urgently please feel free to create a PR and I'll to my best to look at it ASAP.

jmmshn commented 1 year ago

Resolved in PR #82

Example usage: https://github.com/materialsproject/pymatgen-analysis-defects/blob/aff3f20d4697867672a0665caa3fa5d5364b5e29/tests/test_generators.py#L84-L91