Skip to content

HGH cleanup #1064

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

Open
raphaelzstone opened this issue Feb 17, 2025 · 3 comments
Open

HGH cleanup #1064

raphaelzstone opened this issue Feb 17, 2025 · 3 comments

Comments

@raphaelzstone
Copy link
Contributor

Here are all the files that I would need to change to phase out hgh pseudopotential files. Some of the actions I need to take I'm not totally sure about, so some direction would be appreciated based on what I have down for each of the files. At this point I've mostly just noted which files have hgh mentions that need to be reconciled, and took a guess about what or how to reconcile that issue, but I may not totally understand how hgh and gth files work with DFTK. Please advise or confirm for my approach.

test/PspHgh.jl – Probably just needs the file name changed to PspGth, and all references would need changing too.
test/list_psp.jl – I think just change the name to GTH in line 1.
src/pseudo/PspHgh.jl – Maybe a bit complicated. Not totally sure. This probably also encodes for gth files. Not totally sure.
src/pseudo/load_psp.jl – Remove .hgh file abilities.
src/workarounds/forwarddiff_rules.jl – Calling Psphgh, fixed with test/psphgh.jl renaming/structuring.
data/psp/hgh – Just remove all the pseudopotentials? Or maybe just change the names to gth? Not really clear on this.
examples/pseudopotentials.jl – Alter the example so it doesn't use .hgh files (only example with .hgh reference).
docs/features.md – Remove HGH mention (only mention of hgh in docs).

@mfherbst
Copy link
Member

mfherbst commented Feb 18, 2025

Thanks for reaching out ! But I am not sure I fully understand your description.

The data/psp/hgh files are essentially only provided for backward compatibility. The recommended way to use Goedecker-type pseudos is already via PseudoPotentialData.jl where they are already called "gth" files. The extension is essentially arbitrary. In our ecosystem "something.hgh" is the same format as "something.gth", namely the format used by CP2K. But there are in general no file format conventions for such files and people do random things.

Yes, in DFTK we still use the acronym "HGH" at plenty of places and it would probably better to rename this consistently to "GTH", I agree with that. Would that be what you are after ?

@raphaelzstone
Copy link
Contributor Author

Ok, that makes sense. So should data/psp/hgh have the name changed for consistency? and should the examples/pseudopotentials.jl file keep the hgh example it currently uses, or should both examples be from PseudoPotentialData.jl? So then an edited list to change the HGH acronym to be GTH for consistency. Let me know if this looks good.

test/PspHgh.jl – Change file name and references to this file
test/list_psp.jl – I think just change the name to GTH in line 1.
src/pseudo/PspHgh.jl – Change name to GTH and all references
data/psp/hgh – change folder name to GTH? Or should I just leave this one be?
examples/pseudopotentials.jl – Alter the example so it doesn't use .hgh files?
docs/features.md – Remove HGH mention (only mention of hgh in docs).

@mfherbst
Copy link
Member

should data/psp/hgh have the name changed for consistency?

No this would be a breaking change which makes no sense for a feature that will get removed soon-ish anyway. Maybe even the rest of the PR is breaking and then we would probably remove it right now.

The rest looks broadly good to me. Let's discuss further concretely in a PR.

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

No branches or pull requests

2 participants