-
Notifications
You must be signed in to change notification settings - Fork 42
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
Allow more transparent modification of damping parameters #98
Comments
I agree that compared to dftd3 that used either the I wouldn't mind the dftd3 API demanding either func or full parameters and erroring at both. Though that's not the only solution. At the psi4 level, I think it's worth keeping user ability to selectively overwrite a functional's default params. Especially since for dftd4 params that are approximately fixed (alpha, s6 for non-DH, iirc) are in the complete parameter list. Being able to query what parameters just ran from a calc object returned by dftd4 would be great. I tried to probe this once but got back opaque capsule, as you mentioned above. This would let the qcng harness (my part at least) skip the call to the great resolver fn that "reconciles" func and parameters to see if an official functional label can be applied to the results. Thanks for bringing up this topic. |
I should note that the returned object is actually polymorphic and doesn't have to be a rational damping function. Kinda superfluous since we only support one damping function in Accessing the damping parameters returned from |
Let's clarify user here, because I'm mainly thinking API users (like psi4). How DFT-D4 is exposed in psi4 to the actual end user is not necessarily a concern of the API in To provide the ability to selectively overwrite functional default parameters, I don't think |
I completely agree. The less logic the dftd4 API applies behind the scenes, the simpler it is to implement more involved logic like overwriting parameters in psi4. Looking up the values in The API consistency problem I got myself into is long ago creating a I guess I can see leaving the dftd4 API as-is at func-trumps-params in keeping with AtomicInput. Or the only-func or only-params routes in keeping with dftd3. But whichever you choose, I think it's the dftd4 harness's role to follow it and not overwrite the interface. |
Just a quick followup on the possibility of implementing this in the C API. I started a thread in the Fortran discourse on the best way to handle polymorphic objects in a C API situation: https://fortran-lang.discourse.group/t/exposing-polymorphic-objects-to-c/1290 While there seems to be a way, the overall recommendation is to reconsider the API. I'm not going to risk API breakage for this feature, but there might be another way. One option would be to add a new C API call to retrieve damping parameters from the internal storage and use this function to implement the I'll mark it as |
I'm planning to slightly change the behaviour of the damping function constructor in #137 and make it an error to provide both method name and explicit damping parameters. |
Is your feature request related to a problem? Please describe.
API objects handling damping parameters are currently opaque, i.e. once created it is unknown which damping parameters they actually contain.
The API allows two ways to construct damping parameters right now, once via a method name or via a complete set of damping parameters (s6, s8, s9, a1, a2, alp). The Python API is lumping the two ways together and implements an ad hoc selection here, preferring the method name over custom, maybe incomplete, parameters:
dftd4/python/dftd4/interface.py
Lines 135 to 155 in 3e16f9f
Describe the solution you'd like
Not sure, allow to query the object?
Describe alternatives you've considered
Make the object completely transparent (
bind(C)
derived type).Additional context
The API object for the damping parameters must never be partially constructed, i.e. only containing some damping parameters and not all. Also, there is little sense in requesting a method and overwriting its s8 parameter blindly, in this case the user is better aware of all damping parameters and has to take full responsibility for constructing the API object.
The text was updated successfully, but these errors were encountered: