-
Notifications
You must be signed in to change notification settings - Fork 53
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
fix logic error in NEB vacancy supercell generation #246
Conversation
Thanks. Can you add a simple unittest pls? |
Pull Request Test Coverage Report for Build 1139703774
💛 - Coveralls |
Sure thing. Looks like I need to run black formatting too. Before you accept my changes, I wanted to point out that this changes how some supercell structures are generated when vac_mode=False. In reality, I think there are actually three modes that should be considered when making supercells:
The difference between 2 and 3 won't be seen in the majority of structures, but it may be important for lower-symmetry ones. |
There are still some linting errors. |
@shyuep for future reference, what commands do you use to run all linting locally? I'm only familiar with black and don't typically use pylint. Is it just something like |
The commands are listed in this Github workflow: https://github.com/materialsvirtuallab/pymatgen-analysis-diffusion/blob/master/.github/workflows/linting.yml |
Awesome, thank you! |
This pull request address the bug in #245 where the incorrect supercell is generated with vac_mode=True