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

fixed upfs for all current elements #105

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bkincaid256
Copy link
Contributor

@bkincaid256 bkincaid256 commented Apr 19, 2024

This fixes the upfs to be in the format of QE 7.1. This regenerated all but one upf using the exact same inputs as the original UPFs, due to the differences between the computer used this time vs when they were originally generated, the diffs look large, but they are out at the 14-16th decimal place. The energy from the rpts are unchanged, they look like:

diff recipes/Ag/ccECP/Ag.ccECP.AREP.rpt ../work/Ag/ccECP/Ag.ccECP.AREP/Ag.rpt 155c155

  E_tot =     -289.0035643111 Ry
  E_tot =     -289.0035643110 Ry

which showed a diff of 2.7k lines. To the best of my knowledge there should be no issue with these being used as is. All of the upfs originally generated on my system that were updated, show a diff of just the lines that were altered to the new format.

Lastly, Tb SOREP had problems I couldn't pin down when run in opium, it happened no matter if I ran it on PERLMUTTER or on my personal system, whether it was run on the old version opium or not. I wasn't able to get the calculation to finish. I simply updated the format for that one by hand, it wasn't rerun.

closes #89

@prckent
Copy link
Contributor

prckent commented Apr 24, 2024

Hi Ben -

Prior to merging this i made a test calculation for bulk Al. The energies are a little bit different, and by just enough to potentially show up in the last digits of a result table. What are your thoughts for the possible origins of the difference? Inputs & outputs are attached.
Al_test.tar.gz

$ grep "Execution Date" Al.ccECP.upf_*
Al.ccECP.upf_new:#Execution Date : Fri Apr 19 14:16:08 2024
Al.ccECP.upf_orig:#Execution Date : Thu Jun 30 16:16:13 2022
$ grep '^!  ' *.out*
scf.out_new:!    total energy              =      -4.13340513 Ry
scf.out_orig:!    total energy              =      -4.12958855 Ry

@bkincaid256
Copy link
Contributor Author

bkincaid256 commented Apr 24, 2024 via email

@bkincaid256
Copy link
Contributor Author

bkincaid256 commented Apr 24, 2024 via email

@prckent
Copy link
Contributor

prckent commented Apr 24, 2024

OK. Please make that list -- we should craft a sentence to put in the news and updates part of the website. e.g.

"2024-04-24: Updated all UPF files for the new QE7.1 format. All of the UPF files were regenerated. As part of the update we changed the local channels for X, Y, Z, for better overall accuracy. Total energies should only change a small amount. Contact us if that is not the case."

@aannabe
Copy link
Contributor

aannabe commented Apr 24, 2024

@bkincaid256 Thanks for the push. I agree with @prckent that if there is a significant change in the total energies, it needs to be publicized. For example, some time ago, I added energies.txt for each ECP, which shows isolated atomic energies from various codes. The QE numbers there would then be outdated. Another option would be to leave the local channel the same since the improvement in transferability seems marginal.

Additionally, we should also update the *.rpt files. This will make the reference state clear to the user and good for reproducibility.

@bkincaid256
Copy link
Contributor Author

The two ecps affected by the change in local channel are Al.ccECP.upf, and Si.ccECP.upf. I can revert them if that is more convenient, also for all but these two the energies in the rpts are virtually identical. I have generated a full list of diffs between the old rpts and the new, pretty much every discrepancy looks to be machine precision differences. I can update them though as it won't take more than adding a line in my script. There were a few test configurations that had to be removed due to not converging on this machine for whatever reason. I worry about losing that information when the upf should still perform the same on it, opium just had some hiccup with it this time.

@aannabe
Copy link
Contributor

aannabe commented Apr 24, 2024

OK, so if the local channel changes are only in Al and Si, and if the transferability improvement in Si is also marginal, I am inclined towards keeping them unchanged for now (unless we find a good reason to change later). Note that we have published papers using both of these ECPs where we report the SCF totals (diamond Si and GCTA Al).

*rpt file changes are indeed not important if the changes are machine precision.

@bkincaid256
Copy link
Contributor Author

I reversed the change to the two upfs. If someone could verify the energy is giving the old result, that would be very much appreciated.

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.

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