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

Rationalise pseudopotential types #202

Open
davidbowler opened this issue Jun 27, 2023 · 3 comments
Open

Rationalise pseudopotential types #202

davidbowler opened this issue Jun 27, 2023 · 3 comments
Assignees
Labels
area: main-source Relating to the src/ directory (main Conquest source code) improves: stability Fix or enhance issues with stability or robustness needs: confirmation Bug or issue needing confirmation priority: minor time: days type: question Issue to be discussed by developers

Comments

@davidbowler
Copy link
Contributor

At present we have a redundant pseudopotential type (OLDPS) which I think that we should remove. It would also be possible to change the code so that the pseudo_type variable becomes species-dependent, to allow mixing of Conquest and Siesta ion files.

@tsuyoshi38 @lionelalexandre @ayakon Do you have any opinions, particularly on allowing mixing of types?

@davidbowler davidbowler added area: main-source Relating to the src/ directory (main Conquest source code) improves: stability Fix or enhance issues with stability or robustness needs: confirmation Bug or issue needing confirmation priority: minor time: days type: question Issue to be discussed by developers labels Jun 27, 2023
@davidbowler davidbowler self-assigned this Jun 27, 2023
@davidbowler
Copy link
Contributor Author

Looking more carefully at this, it's clear that we can mix pseudopotential types when we use the neutral atom potential. However it's harder if we don't: the calculation for the local part of the potential might cause problems with mixed types. Do we want to allow mixed types only with neutral atom potential?

@tsuyoshi38
Copy link
Contributor

I agree that removing OLDPS is okay. (As far as I remember, it was easy to make compatible with other pseudo potentials like the ones used for STATE with OLDPS. But, we don't have PAOs for this.)

For mixing Siesta and Conquest ion files,
since the expression of local potential is different, it is difficult to mix them without neutral atom (NA) potentials, but should be okay with NA. (as you said. Sorry, I have not noticed your comment above.. > Dave.)
I can imagine that it would be useful in some cases if we can mix them, and
it should be okay to assume users need to use NA potential in this case, I think.

@davidbowler
Copy link
Contributor Author

Thanks @tsuyoshi38 I'll go ahead with some changes on this basis (but I'll add a test for mixed types and non-neutral atom).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: main-source Relating to the src/ directory (main Conquest source code) improves: stability Fix or enhance issues with stability or robustness needs: confirmation Bug or issue needing confirmation priority: minor time: days type: question Issue to be discussed by developers
Projects
None yet
Development

No branches or pull requests

2 participants