search

QMCPACK / pseudopotentiallibrary

Repository for PseudopotentialLibrary.org website and database
https://pseudopotentiallibrary.org
12 stars 14 forks source link

Naming convention error for DFT+U calculations in QE 7.1 for Fe.ccecp-soft.upf #89

Open jptowns opened 1 year ago

jptowns commented 1 year ago

In QE 7.1 the DFT+U input structure was changed. Now one has to specify a (n,l) channel on which to apply U/J. E.g. "Fe-3d". It seems that the code actually looks for "3d" somewhere in the pseudopotential. But the NCSU seems to use a more general "S/P/D" naming convention that causes QE 7.1 to yield an error. The problem seems to be here:

<PP_HEADER>
   0         Version Number
   Fe        Element
   NC        Norm - Conserving pseudopotential
    F      Nonlinear Core Correction
SLA  PW   PBE  PBE     PBE  Exchange-Correlation functional
 16          Z valence
 0          Total energy
 0.000000   0.000000     Suggested cutoff for wfc and rho
 2           Max angular momentum component
 1160           Number of points in mesh
 3  2     Number of Wavefuncitons, Number of Projectors
 Wavefunctions         nl  l   occ
                       S  0  2.000000
                       P  1  6.000000
                       D  2  6.000000       <<<<<<<====== Changing this from "D" to "3D" fixes the error
</PP_HEADER>

Changing the "D" under 'wavefunctions' to "3D" seems to fix the problem. It may be worth changing to avoid this error for other transition metals.

romanfanta4 commented 3 months ago

Hello, Do you have any updates regarding this issue?

prckent commented 3 months ago

We could do a search and replace to fix this issue. @jptowns @bkincaid256 Any preferences? I assume QE <7.1 is still OK.

bkincaid256 commented 3 months ago

I wasn't aware of this problem. It seems to be a holdover from how we generate the various ccECP formats in opium. I imagine a simple find and replace is sufficient for now. I'll look into where the formats are being generated to see about fixing it from being a problem in future elements/runs. Does this mean that the two inputs are incompatible between QE 7.1 and <7.1? If so, what do we need to do for compatibility?

prckent commented 3 months ago

I think find . -name "*.upf" -exec sed -ibak "s/ D 2 /3D 2 /g" {} \; would be enough if there are not any compatibility issues.

prckent commented 3 months ago

Will merge #103 Tuesday afternoon unless there are comments to hold off.

romanfanta4 commented 3 months ago

I’ve tested the updated pseudopotentials (with tweaks in the s, p, and d channels) with QE 7.0, and everything’s working fine. This means the new naming system is ready to go, and there should be no backward compatibility issues. For future-proofing, all channels need to be changed (not just D): ‘P’ becomes ‘3P’, ‘D’ becomes ‘3D’, ‘S’ becomes ‘4S’, and so on. My testing files are in the attachment: test-files.tar.gz

prckent commented 3 months ago

@bkincaid256 How painful would it be to generate UPF2 format for everything? I am quite tempted to close the PR since changing all the channels by hand will be error prone.

bkincaid256 commented 3 months ago

It shouldn't be too awful in theory. I just need to find where the modified code that Cody put into opium does the UPF generation and make sure that it preappends the n quantum number to the l channels. Once I have that it would be an afternoon of running the inputs or so. I just haven't looked into it yet. I'll give it a look today.

camelto2 commented 3 months ago

I also have a separate tool, python based, but similar implementation to ppconvert, which writes to UPF2 and should have the correct labeling. It also does SOC, so it could entirely replace opium. The only downside is that it does not currently have ghost testing, which opium does, but that could be added. It is a bit more stable than opium, and I've done a little bit of work to make it handle multiple projectors. If this is something we want to look into, I can spend some time polishing it a bit to make sure it is ready to go

prckent commented 3 months ago

Interesting. Thanks Cody. Since there is already a recipe for making the UPFs, can we skip ghost testing? Or is the recipe used different enough from Opium that you think ghost testing is mandated? We can spot check the generated data.

bkincaid256 commented 3 months ago

My concern with the tool is that in my testing, using the same ECP on the same reference state led to pretty discrepant performance between opium and the tool(mainly looking at cutoffs). The tool yielded suspiciously soft UPFs. At the time, I didn't see a way to try it against several states and check the performance like I can in opium. I could work out a way to test a batch of states and confirm agreement, but I would want to make sure the tool is giving reliable results. My cutoff testing led to differences of around a factor of 2, so the upfs are pretty different between the codes.

camelto2 commented 3 months ago

FWIW, I actually trust the cutoff calculations in the python tool more. I have noticed strange behavior from opium when calculating the cutoffs, i.e. blips in the convergence rather than a smooth convergence. The python tool does not show this and always has clean convergence with cutoff.

Also, the cutoffs given by either tool aren't technically trustworthy anyway. The required cutoff on an isolated atom in free space (what we calculate in both tools) is not necessarily the same as the cutoff in a condensed environment. You always need to test anyway. The atom cutoffs just give you a rough idea of where to start searching.

camelto2 commented 3 months ago

The testing of multiple states is something I need to work on.

bkincaid256 commented 3 months ago

The cutoff testing was done by isolating an atom in a large box and changing the cutoff until the energy difference was below a given threshold. This was all done in QE, I wasn't meaning that I was relying on the cutoff given by opium, but the point remains. The two give very different results even when using the same functional. I would switch to the tool assuming I could compare how well the generated pseudopotential did at recreating gaps among states, it gives the favorable cutoff.

bkincaid256 commented 3 months ago

For now, I may have a fix for the opium generation of UPFs. I will need to test it on other heavier elements to make sure it labels everything correctly.

UPDATE: The fix works for spin-orbit at the moment, but for whatever reason the numbering is done in some local indices for nonspin-orbit calculations. The primary quantum number resets in the non-SO case.

bkincaid256 commented 3 months ago

I managed to get it fixed at the generation step in opium for both SO and AREP. I'll put together a pull request with the fixed upf files soon.

prckent commented 2 months ago

How is this going?

bkincaid256 commented 2 months ago

I was working on this the last couple days when I had some time. It’s mostly ready there are 6-7 upfs that don’t generate despite being run with identical input to when they were made. The rest work and of the ones spot checked, match the old upf save the change needed. I probably will have it sent today.

On Fri, Apr 19, 2024 at 10:42 AM Paul R. C. Kent @.***> wrote:

How is this going?

— Reply to this email directly, view it on GitHub https://github.com/QMCPACK/pseudopotentiallibrary/issues/89#issuecomment-2066730138, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOSTPRC63HWFYL65TWSUF2LY6EUN5AVCNFSM6AAAAAAVSAA6JGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRWG4ZTAMJTHA . You are receiving this because you were mentioned.Message ID: @.***>