-
Notifications
You must be signed in to change notification settings - Fork 463
Adds r2SCAN hybrids; r2SCAN-3c and B97-3c #2842
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
Conversation
Where are the basis sets from? Ideally they should just go straight to https://github.com/MolSSI-BSE/basis_set_exchange The r2SCAN hybrids are available in the current version of Libxc. |
I took the basis from ORCA ( |
Hi @hokru, I uploaded Locally, this passes with
|
@hokru I get the error below when there's no dftd4 of any kind around. (It's what Azure linux is doing now.) When I change the try/except in qcng empirical_disp_param line 906 from ModuleNotFoundError to ImportError. This is py310, so I'm not sure why, but it might be a sol'n.
|
I've tweaked the https://github.com/MolSSI/QCEngine/pull/393/files PR slightly, and I think it's ready for merge. Here, I've added a few tests (b97-3c, r2scan2-3c, and r2scan2-d4) and tried them with several combinations of old/new gcp, old/new dftd3, and 3.4/3.5 dftd4, so I'm semi-confident they run when they ought and give helpful errors when they can't. The tests I added cover energy and gradient, but they're purely internal -- no reference values from a separate implementation. What further tests are wanted? The labels/selectors for pytest should be in good shape (that is, tests will skip if wrong gcp/d3/d4 detected). CTest selectors aren't in good shape, so if you get helpful "can't run" errors, that's expected; wrong values aren't expected. |
#URL https://github.com/MolSSI/QCEngine/archive/v0.28.0.tar.gz | ||
#URL https://github.com/hokru/QCEngine/archive/3cupdate.tar.gz | ||
URL https://github.com/loriab/QCEngine/archive/v0.28.0.dev2.tar.gz # == hokru:3cupdate but tag needed for pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#URL https://github.com/MolSSI/QCEngine/archive/v0.28.0.tar.gz | |
#URL https://github.com/hokru/QCEngine/archive/3cupdate.tar.gz | |
URL https://github.com/loriab/QCEngine/archive/v0.28.0.dev2.tar.gz # == hokru:3cupdate but tag needed for pip | |
#URL https://github.com/MolSSI/QCEngine/archive/v0.28.0.tar.gz | |
URL https://github.com/MolSSI/QCEngine/archive/v0.28.0.dev3.tar.gz # includes hokru:3cupdate; tag needed for pip |
when resuming work on this PR, switch to this tag if v0.28 not yet minted. I don't want to trigger the full CI or I'd push it.
The basis sets were merged to BSE in MolSSI-BSE/basis_set_exchange#276 but some issues are still fixed in MolSSI-BSE/basis_set_exchange#282 One should use the basis sets from the BSE. |
"name": "R2SCAN0", | ||
"x_functionals": { | ||
"MGGA_X_R2SCAN": {"alpha": 0.75} | ||
}, | ||
"c_functionals": { | ||
"MGGA_C_R2SCAN": {} | ||
}, | ||
"x_hf": { | ||
"alpha": 0.25 | ||
}, | ||
"description": ' r2SCAN0 Hybrid Meta-GGA XC Functional\n', | ||
"citation": ' M. Bursch, H. Neugebauer, S. Ehlert, S. Grimme J. Chem. Phys. 156, 134105, 2022 \n', | ||
"doi": "10.1063/5.0086040", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use XC_HYB_MGGA_XC_R2SCAN0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now we're supporting libxc v.5.1.2 ... v6.x . Would that range have to be curtailed to use the all-in-one fctl definitions?
iirc, we ended up supporting two major versions because v6 came out right before a psi4 release, and we didn't want to demand a sudden change. That is, there's no harm in using only v6 now that c-f can build new Windows versions for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the r2scan hybrids were added in 6.1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's plan on dropping v5 and requiring minimum v6.1 then.
"name": "R2SCANh", | ||
"x_functionals": { | ||
"MGGA_X_R2SCAN": {"alpha": 0.90} | ||
}, | ||
"c_functionals": { | ||
"MGGA_C_R2SCAN": {} | ||
}, | ||
"x_hf": { | ||
"alpha": 0.10 | ||
}, | ||
"description": ' r2SCANh Hybrid Meta-GGA XC Functional\n', | ||
"citation": ' M. Bursch, H. Neugebauer, S. Ehlert, S. Grimme J. Chem. Phys. 156, 134105, 2022 \n', | ||
"doi": "10.1063/5.0086040", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use XC_HYB_MGGA_XC_R2SCANH
"name": "R2SCAN50", | ||
"x_functionals": { | ||
"MGGA_X_R2SCAN": {"alpha": 0.50} | ||
}, | ||
"c_functionals": { | ||
"MGGA_C_R2SCAN": {} | ||
}, | ||
"x_hf": { | ||
"alpha": 0.50 | ||
}, | ||
"description": ' r2SCAN50 Hybrid Meta-GGA XC Functional\n', | ||
"citation": ' M. Bursch, H. Neugebauer, S. Ehlert, S. Grimme J. Chem. Phys. 156, 134105, 2022 \n', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use XC_HYB_MGGA_XC_R2SCAN50
Hi folks - I'd like to help with this, although I'm not familiar with the psi4 code internals. Do you need reference values for the tests? (on what systems?) I'm wondering if the gcp part of r2SCAN-3c would affect the results in any test with a single molecule; do we need something like a water dimer? |
Thanks for your interest and the offer, @aizvorski. Looking back over this, the necessary QCEngine has now been released, so it looks like my first steps are updating that CI and limiting Libxc to v6 so the functional definitions can be simplified. Certainly intermolecular tests are desirable. There's some ref values here http://www.thch.uni-bonn.de/tc.old/downloads/GMTKN/GMTKN55/functional/r2SCAN-3c.html but largely they need hunting down in either papers or an independent implementation. I like S22 but anything independent will do. Then Susi had some basis set comments that I haven't looked into yet. |
@aizvorski or @hokru (or anyone), did you happen to have on hand independent ref values for any of these five fctls, particularly r2SCAN-3C & B97-3C ? (Note that c-f dftd4 isn't available for py312, so I'll likely need to adjust the CI to improve testing coverage. If this makes it into v1.9, I'm good with dropping support for dftd3 classic and the psi4 build of dftd4. Also, the plan is libxc=6, dropping v5 support.) |
This adds external comparisons for the r2SCAN hybrids, r2scan-3c, B97-3c. It also implements wB97X-3c. I think it's good to go. (well, CI/test-skipping may yet need tweaks.) But @susilehtola or @hokru might want to look it over. |
# "GGA_XC_B97_3C": {} #for libxc >= v6.0 | ||
"GGA_XC_B97_D": { | ||
"tweak": { # needed until libxc >= v6.0 | ||
"_cx0": 1.076616, | ||
"_cx1":-0.469912, | ||
"_cx2":3.322442, | ||
"_css0":0.543788, | ||
"_css1":-1.444420, | ||
"_css2":1.637436, | ||
"_cos0":0.635047, | ||
"_cos1":5.532103, | ||
"_cos2":-15.301575, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be dropped, no?
doc/sphinxman/source/dftd3.rst
Outdated
@@ -222,6 +223,7 @@ available only through the ``DFTD3`` or ``DFTD4`` programs. Once installed, the | |||
``dftd3``/|PSIfour| and ``dftd4``/|PSIfour| interfaces are transparent, and all corrections are | |||
interfaced exactly alike. | |||
The -D3 interface can use classic or simple-dftd3 programs interchangeably and will prefer the latter. | |||
Starting in v1.9, the classic program is no longer supported or tested, though it won't be deliberately disabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting in v1.9, the classic program is no longer supported or tested, though it won't be deliberately disabled. | |
Starting in v1.9, the classic program is no longer supported or tested, though it isn't deliberately disabled. |
Slightly more grammatically consistent with my suggestion imo, but feel free to ignore if you don't agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better since I don't want to promise to never disable the classic. I'll add it to a different PR if that's the only change, since CI is already complete here.
@@ -99,6 +99,11 @@ std::shared_ptr<SuperFunctional> SuperFunctional::XC_build(std::string name, boo | |||
sup->set_x_alpha(xc_func->global_exchange()); | |||
sup->set_x_beta(xc_func->lr_exchange()); | |||
|
|||
// User tweakers | |||
if (tweakers_->size()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a check here for the case that tweakers_
is uninitialized, i.e., std::nullopt
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual use of the tweakers for this PR is all removed by now by requiring libxc v6, but it's good to future proof. What do you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, got it! I suppose that's a new feature of the code I missed. But regarding suggestions, something like:
if (tweakers_.value().size()) {
Will cause the code to throw an exception if tweakers_
is uninitialized, acting as the check. If you want to avoid throwing entirely, you could instead enclose this if
statement in another if
-block:
if (tweakers_.has_value()) { ... }
to ensure that tweakers_
must be initialized for the set_tweak
call to proceed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, fixed. One doesn't want the has_value version b/c that always tries to set tweak and some/most? libxc fctls aren't tweakable.
Description
Adds new density functional approximations and Grimme group composite methods.
closes #2121 #1898
User API & Changelog headlines
r2SCAN0
,r2SCANh
,r2SCAN50
(plus-D4
versions)r2SCAN-3C
,B97-3C
def2-mTZVP
,def2-mTZVPP
Dev notes & details
B97-3c
requiress-dftd3
andmctc-gcp
r2SCAN-3c
requiresmctc-gcp
anddftd4 3.5.0
[dispersion]['params']
functional dictionary can be incomplete. Defaults will be added automatically.qcengine
update update gCP levels and extend D4 defaults MolSSI/QCEngine#393 (review)dftd4-python
with Allow passing through values for dispersion model dftd4/dftd4#184Checklist
Status