R2scan - Githubissues

JFurness1 commented 4 years ago

Description

We recently published a revised SCAN meta-GGA functional that addresses its numerical inefficiencies. The resulting functional "r2scan" preserves the good accuracy of SCAN with improved numerical performance. This is achieved by restoring exact constraint adherence (and consequently transferable accuracy) to the "regularised-SCAN" functional of Bartok and Yates. The pre-print manuscript can be found at arXiv:2008.03374, with the full publication in JCPL accepted 02 Sep 2020.

This addition is made as a general "SCAN_like_eps.hpp" header implementing the SCAN functional and taking integer flags to include or exclude modifications as appropriate. Each functional has its own x/c source file which calls the exchange/correlation functions with appropriate flags.

The PR includes 2 additional as yet unpublished functionals "r++SCAN" (rppscan) and "r4SCAN". These will be described in an upcoming further publication but are included as they come "for free" given the shared code structure of the header file.

Regards, James Furness

Motivation and Context

This adds the SCAN and revised SCAN functionals to the library.

How Has This Been Tested?

These additions were originally made in the XCFun that ships with Turbomole V 7.4. I confirmed that this XCFun implementation agrees with the Turbomole Fortran funcitonal routines and their analytical derivatives to all printed figures. The Fortran Turbomole implementation was the definitive version during development.

However, porting these additions directly to a freshly cloned copy of XCFun, they appear to cause the test to fail, where the fresh clone passes. A log of this failure is included.

I cannot determine if this is a problem with my implementation, or a problem with how I have included it in the testing suite. I note that my Turbomole XCFun implementation runs flawlessly, and these files are copied directly, so I suspect the latter.

Please advise and I will fix, the test log file is included.

LastTest.log

Screenshots (if appropriate):

Todos

User-Facing for Release Notes
- [x] New functionals available for use.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Questions

[x] Please see comments regarding issue with testing.

Status

[ ] Ready to go

robertodr commented 4 years ago

Thank you so much for this contribution! I will look into the failing tests.

bast commented 4 years ago

Thanks James!

bast commented 4 years ago

On my todo list to solve the testing problem. With Roberto we expect this to be a minor issue. Soon :-)

bast commented 4 years ago

Finally looked at it. The problem seems to be that these functionals are added to the test set but they have no reference data and then the code perhaps assumes zeros as input.

One workaround is to remove these lines:

index 8982275..7d66b7c 100644
--- a/src/functionals/rppSCANc.cpp
+++ b/src/functionals/rppSCANc.cpp
@@ -18,6 +18,4 @@ FUNCTIONAL(XC_RPPSCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(rppSCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/rppSCANx.cpp b/src/functionals/rppSCANx.cpp
index a09fc54..2476d6f 100644
:...skipping...
diff --git a/src/functionals/SCANc.cpp b/src/functionals/SCANc.cpp
index c69a899..055a7c3 100644
--- a/src/functionals/SCANc.cpp
+++ b/src/functionals/SCANc.cpp
@@ -17,6 +17,4 @@ FUNCTIONAL(XC_SCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(SCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/SCANx.cpp b/src/functionals/SCANx.cpp
index 66cf57b..f2b7149 100644
--- a/src/functionals/SCANx.cpp
+++ b/src/functionals/SCANx.cpp
@@ -23,6 +23,4 @@ FUNCTIONAL(XC_SCANX) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(SCANX)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/r2SCANc.cpp b/src/functionals/r2SCANc.cpp
index 47f2bef..e7e9315 100644
--- a/src/functionals/r2SCANc.cpp
+++ b/src/functionals/r2SCANc.cpp
@@ -18,6 +18,4 @@ FUNCTIONAL(XC_R2SCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(r2SCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/r2SCANx.cpp b/src/functionals/r2SCANx.cpp
index e293706..23d3be7 100644
--- a/src/functionals/r2SCANx.cpp
+++ b/src/functionals/r2SCANx.cpp
@@ -23,6 +23,4 @@ FUNCTIONAL(XC_R2SCANX) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(r2SCANX)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/r4SCANc.cpp b/src/functionals/r4SCANc.cpp
index d50d8e6..60e81d0 100644
--- a/src/functionals/r4SCANc.cpp
+++ b/src/functionals/r4SCANc.cpp
@@ -18,6 +18,4 @@ FUNCTIONAL(XC_R4SCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(r4SCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/r4SCANx.cpp b/src/functionals/r4SCANx.cpp
index 7712daa..e46a381 100644
--- a/src/functionals/r4SCANx.cpp
+++ b/src/functionals/r4SCANx.cpp
@@ -23,6 +23,4 @@ FUNCTIONAL(XC_R4SCANX) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(r4SCANX)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/rSCANc.cpp b/src/functionals/rSCANc.cpp
index e7ef1f3..8edf1c6 100644
--- a/src/functionals/rSCANc.cpp
+++ b/src/functionals/rSCANc.cpp
@@ -18,6 +18,4 @@ FUNCTIONAL(XC_RSCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(rSCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/rSCANx.cpp b/src/functionals/rSCANx.cpp
index 4960d3d..a79f72d 100644
--- a/src/functionals/rSCANx.cpp
+++ b/src/functionals/rSCANx.cpp
@@ -23,6 +23,4 @@ FUNCTIONAL(XC_RSCANX) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(rSCANX)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/rppSCANc.cpp b/src/functionals/rppSCANc.cpp
index 8982275..7d66b7c 100644
--- a/src/functionals/rppSCANc.cpp
+++ b/src/functionals/rppSCANc.cpp
@@ -18,6 +18,4 @@ FUNCTIONAL(XC_RPPSCANC) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(rppSCANC)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };
diff --git a/src/functionals/rppSCANx.cpp b/src/functionals/rppSCANx.cpp
index a09fc54..2476d6f 100644
--- a/src/functionals/rppSCANx.cpp
+++ b/src/functionals/rppSCANx.cpp
@@ -23,6 +23,4 @@ FUNCTIONAL(XC_RPPSCANX) = {
   "Implemented by James Furness\n",
   XC_DENSITY | XC_GRADIENT | XC_KINETIC,
   ENERGY_FUNCTION(rppSCANX)
-  XC_A_B_GAA_GAB_GBB_TAUA_TAUB,
-  XC_PARTIAL_DERIVATIVES,
 };

This will make the testset pass since these functionals are then skipped.

Another solution is to add reference data to the tests like for example here: https://github.com/dftlibs/xcfun/blob/3751220a382ec02601103dc5eaec8525a5418bf4/src/functionals/pbec.cpp#L68-L79

JFurness1 commented 4 years ago

Great. I'll make some reference data and add it as per the pbec example.

I should be able to get to it in the next day or so. Thanks for the advice!

bast commented 4 years ago

Thanks a lot James! After this is in, we can release a new version. This is a great addition.

JFurness1 commented 4 years ago

I'm looking at finding these test values now.

Looking at the pbec.cpp as an example, can you give any guide on the numbers? I have been able to work out that:

2,                       // ? Maybe 1 = LDA, 2 = GGA, 3 = mGGA?
1e-11,                 // Precision required?                                                
{0.39E+02, 0.38E+02, 0.81E+06, 0.82E+06, 0.82E+06},             // {d0, d1, g00, g11, g01} presumably needs {t0, t1} appended for mGGA       
{               // Presumably this is the 0th and higher order derivatives? 
                // Is there a structure for what order of derivative and with respect to what?
    -0.184442072405E+01, -0.814334534280E-01, -0.820182123795E-01,     
    0.510839298939E-06,  0.102167859788E-05,  0.510839298939E-06,      
    -0.124297349784E-02, -0.183505806584E-02, 0.134850158624E-07,      
    0.269700317248E-07,  0.134850158624E-07,  -0.125767116982E-02,     
    0.136189478240E-07,  0.272378956480E-07,  0.136189478240E-07,      
    -0.216571369852E-12, -0.433142739704E-12, -0.216571369852E-12,     
    -0.866285479407E-12, -0.433142739704E-12, -0.216571369852E-12,     
}};

I would also like to ask if there is a guide for interfacing with python. I can see that python bindings are built by adding --pythonbindings to cmake, but when I try to import the library I find,

File "/Users/james/QC_Codes/XCFun/python/xcfun.py", line 17, in <module>
    from ._xcfun import *
ImportError: attempted relative import with no known parent package

So I guess something hasn't built properly?

Many thanks, James

JFurness1 commented 4 years ago

Ah, I can see from the referenced: http://www.cse.scitech.ac.uk/ccg/dft/data_pt_c_pbe.html how the derivatives should be structured. Maybe then the first number "2" is 2nd order.

robertodr commented 4 years ago

Hi James, how are you compiling XCFun? And how are you trying to import the Python module? I've just done:

./setup --pybindings

and then

cd build/lib/python
python -c "import xcfun; print(xcfun.xcfun_splash())"

If you want to import xcfun from anywhere else, you'll have to set the PYTHONPATH explicitly:

env PYTHONPATH=<build_dir>/lib/python:$PYTHONPATH python -c "import xcfun; print(xcfun.xcfun_splash())"

robertodr commented 4 years ago

Yes, adding tests is not very straightforward. XCFun uses C++ and the C preprocessor to do metaprogramming, so many things are not always transparent.

Let's see if I manage to explain. I'll take the pbec.cpp file and the snippet that defines PBE correlation as example. The FUNCTIONAL macro (in functional.hpp) will fill a functional_data plain-old data (POD) structure (in xcint.hpp) with the stuff in braces after the equal sign. The fields of this POD are:

short_description
long_description
depends which variables the functional uses (density and gradient for PBE correlation)
the energy function, this is more C preprocessor trickery :mage: :sparkles: :boom:
test_vars the variables to use when testing: XC_A_B_GAA_GAB_GBB
test_mode the mode to use when testing: XC_PARTIAL_DERIVATIVES
test_order 2 in this case, so up to second derivatives
test_threshold
test_in array of input parameters for the test, in the order give by test_vars. This array can contain at most 16 elements. This limit can be lifted by changing the second template argument to std:array on line 61. However, as long as you don't add new variables, an array of length 16 should be enough.

test_out array of output quantities from the test, in the graded reverse lexicographical order. This array can contain at most 128 elements. This limit can be lifted by changing the second template argument to std::array on line 62.

FUNCTIONAL(XC_PBEC) = {
"PBE correlation functional",  // ==> **short_description**
"PBE correlation functional.\n"
"J.P. Perdew, K. Burke, and M. Ernzerhof, Generalized\n"
"gradient approximation made simple, " 
"Phys. Rev. Lett. 77 (1996) 3865-3868\n"
"Implemented by Ulf Ekstrom\n",  // ==> **long_description**
// Test case from http://www.cse.scitech.ac.uk/ccg/dft/data_pt_c_pbe.html
XC_DENSITY | XC_GRADIENT,  // ==> **depends**
ENERGY_FUNCTION(pbec) XC_A_B_GAA_GAB_GBB,  // ==> energy function and **test_vars**
XC_PARTIAL_DERIVATIVES,  // ==> **test_mode**
2,  // ==> **test_order**
1e-11,  // ==> **test_threshold**
{0.39E+02, 0.38E+02, 0.81E+06, 0.82E+06, 0.82E+06},  // ==> **test_in**
{
    -0.184442072405E+01, -0.814334534280E-01, -0.820182123795E-01,
    0.510839298939E-06,  0.102167859788E-05,  0.510839298939E-06,
    -0.124297349784E-02, -0.183505806584E-02, 0.134850158624E-07,
    0.269700317248E-07,  0.134850158624E-07,  -0.125767116982E-02,
    0.136189478240E-07,  0.272378956480E-07,  0.136189478240E-07,
    -0.216571369852E-12, -0.433142739704E-12, -0.216571369852E-12,
    -0.866285479407E-12, -0.433142739704E-12, -0.216571369852E-12,
}  // ==> **test_out** };

The output is given in graded reverse lexicographical order see here for a GGA example: https://xcfun.readthedocs.io/en/latest/using.html#input-output-and-units

JFurness1 commented 4 years ago

I think I was trying to import the wrong thing for python. I can import it following your advice, but the API is not so clear to me.

From poking around in the xcfun.py file, it seems like meta-GGA are not supported through this interface?

xc = xcfun.Functional({'SCANc': 1.0})
print(xc.type)
xc.eval_energy_n(nd0+nd1)

prints "2" as expected. But raises an exception for the energy evaluation.

dir() on the module shows a few more options. I've been trying to work out how to call "xcfun_set", but I can't understand what the first argument should be.

I can create the data for the 1st order derivatives from my analytical derivative routines, but I haven't implemented second order terms yet. It would be nice to understand how to call XCFun from python for future development work though!

robertodr commented 4 years ago

The Python interface is supposed to only expose the Functional class, but since I didn't write __init__.py properly, you see the "plumbing" level too, that is, the pybind11 generated bindings to the C++ code.

The problem with eval_energy_n (and all other such functions) is that they were not written for metaGGAs... The Functional class needs to be extended (paging @chjacob-tubs and @aspgomes who are the original authors)

If you want to use the plumbing bindings you should:

# create empty functional
xc = xcfun_new()
# fill it with SCANc
ierr = xcfun_set(xc, "SCANc", 1.0)
# set evaluation variables enum (I'm not sure what you'd need here...)
vars = XC_N_S_GNN_GNS_GSS_TAUN_TAUS 
# set evaluation mode enum (you might want another mode)
mode = XC_PARTIAL_DERIVATIVES
# set the order (0 for energy, etc)
order = 0 
# set up evaluation mode
xcfun_eval_setup(xc, vars, mode, order)
# set input variables as a numpy array
dens = ...
# evaluate, the return value is a numpy array
ret = xcfun_eval(xc, dens)
# clean up functional
xcfun_delete(xc)

JFurness1 commented 4 years ago

Ok, hopefully that's got them all.

Thanks for the advice on running XCFun with python. That will be a very useful development tool for me when functionals are still changing a lot. It should cut down the hours of painful analytic differentiation significantly.

robertodr commented 4 years ago

Thanks for the contribution! And we hope XCFun can indeed be useful for your research also in the future. @bast has started xcauto a pure Python project in the spirit of XCFun (automatic differentiation for XC functionals) using the JAX library for autodiff. You might find that useful too.

dftlibs / xcfun

R2scan #140

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Todos

Types of changes

Questions

Status