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

PseudoPotentialIO #835

Closed
wants to merge 39 commits into from
Closed

Conversation

azadoks
Copy link
Contributor

@azadoks azadoks commented Mar 20, 2023

I released PseudoPotentialIO v0.2.0 this morning, so while everything is fresh, I started to work on a fewest-changes integration of it into DFTK.

I'm still considering a possible interface change on the PseudoPotentialIO side that could help with code duplication and would require a bit of a rework here, but doing this should help shake out any remaining bugs / rough edges in PseudoPotentialIO.

Most everything is working well with relatively minor changes, but there are a couple things that might need to be changed in PseudoPotentialIO to get e.g. ForwardDiff working as expected.

  • Change some typing in PPIO HghPsP for forward diff?
  • Re-export load_psp in DFTK so that PseudoPotentialIO doesn't have to be usinged every time you want to load a pseudo?

@azadoks azadoks changed the title Pseudopotentialio PseudoPotentialIO Mar 20, 2023
@azadoks
Copy link
Contributor Author

azadoks commented Mar 21, 2023

The only test that's failing is the HGH pseudopotential sensitivity using ForwardDiff. I'm not really sure what's going on there; even if I locally modify the PseudoPotentialIO HGH implementation to be basically identical to the one from DFTK, it still doesn't work.

@azadoks azadoks marked this pull request as draft March 21, 2023 14:50
@azadoks
Copy link
Contributor Author

azadoks commented Mar 22, 2023

See azadoks/PseudoPotentialIO.jl#11 for the alternative interface in PseudoPotentialIO

@azadoks azadoks closed this Aug 29, 2023
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

Successfully merging this pull request may close these issues.

2 participants