Added API functions for ASI API implemenation. Including import and e…
This PR is a rebased version of the PR #1093 . It includes all the work done to enable ASI API implementation. Mainly C API extension to include export and import of the Hamiltonian, Overlap, and Density matrices.
Codecov Report
Attention: 2 lines in your changes are missing coverage. Please review.
Comparison is base (
21d2e0a) 69.81% compared to head (99afc33) 69.81%.
:exclamation: Current head 99afc33 differs from pull request most recent head fa83b9a. Consider uploading reports for the commit fa83b9a to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #1217 +/- ##
==========================================
- Coverage 69.81% 69.81% -0.01%
==========================================
Files 227 228 +1
Lines 42543 42382 -161
==========================================
- Hits 29700 29587 -113
+ Misses 12843 12795 -48
| Files | Coverage Δ | |
|---|---|---|
| src/dftbp/api/mm/capi.F90 | 91.11% <100.00%> (+8.44%) |
:arrow_up: |
| src/dftbp/api/mm/mmapi.F90 | 80.54% <100.00%> (+2.32%) |
:arrow_up: |
| src/dftbp/dftbplus/initprogram.F90 | 82.86% <ø> (+0.13%) |
:arrow_up: |
| src/dftbp/dftbplus/main.F90 | 90.86% <100.00%> (+0.01%) |
:arrow_up: |
| src/dftbp/dftbplus/mainapi.F90 | 83.97% <100.00%> (+3.61%) |
:arrow_up: |
| src/dftbp/api/mm/apicallbackc.f90 | 90.00% <90.00%> (ø) |
|
| src/dftbp/dftbplus/apicallback.F90 | 96.00% <96.00%> (ø) |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I believe that all suggestions are accepted or fixed.
@PavelStishenko I've made a PR into your branch (instead of listing lots of minor changes). As mentioned in that PR, there are then a few things to discuss on top of this, mostly to lock down users from performing broken calculations if the matrices are imported.
OK, just to keep them, listing the comments from PR#1 into this:
- the 'invokeX' method isn't too informative as a name for the API calls, is it an exchange, import or export of quantities?
- Please export double precision values for the expdmhs autotest.tag data.
- Cases that will fail to work correctly will need to be locked down (mostly property evaluations using the sparse S, as this isn't constructed, but this could be constructed given the k-dependent S, but we'll need to know the spatial cut-off of matrix elements).
- If the callback gets registered, various invoked modes of the code will need to be locked out of being used (mostly excited state methods).
@PavelStishenko Are there any security issues for the current structure of leaving an open set of calls in the binary? There don't seem to be any obvious buffer overrun possibilities. I guess we can at least add a flag inside the DFTB+ input to disable the callback register and only build it in if the api is enabled.
@PavelStishenko Are there any security issues for the current structure of leaving an open set of calls in the binary? There don't seem to be any obvious buffer overrun possibilities. I guess we can at least add a flag inside the DFTB+ input to disable the callback register and only build it in if the api is enabled.
If I understand your concerns correctly, then for sure there are no security issues due to API functions. This kind of API can be used only when the DFTB+ library is already in the address space of the process that is able to do API calls. In that case the process is capable to do whatever it wants with the library. It is by design. Of course, if there is a bug in the process that loads and uses the DFTB+ library, then it can corrupt anything in its address space, including DFTB+ data, but I don't think that it is a problem of the library.
Perhaps it is possible to compile API support conditionally, but I don't see the problem that is supposed to be solved in that way. It will require more efforts to support and test such code. And definitely will take a time to implement.
The API can be conditionally compiled, but not the ASI parts. I think it is also safer if the user input can also turn the feature on and off for enabled binaries (this is what i-PI does). I'll make a PR into your branch with the changes for this (rebased on the current main code) in a few minutes.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.