mDFTB #1325
mDFTB #1325
Conversation
|
Certainly! I will do that. |
|
@v2quan89 Thanks a lot for the PR, this is a long expected one. 😄 I agree with @bhourahine, the first thing would be to create tests. Once that done, the code should be rebased on current main. And then, we should do some refactoring (mostly packaging things into types and/or subroutines). But let's first have tests with reasonable coverage for the new code. |
|
@bhourahine and @aradi Thank you for your comments. I have just added five test cases, as you suggested. |
There was a problem hiding this comment.
How does this integrate with the existing multipole infrastructure we have in DFTB+? Would it be possible to map the scaling parameters for the momenta to the actual dipole and quadrupole integrals we already store in the codebase? It would be preferable to converge to a single multipole infrastructure and not have two parallel implementations.
|
@awvwgk This multipole is separated from the existing multipole in DFTB+. As far as I know, the existing multipole is from the xTB Hamiltonian; this one is for the DFTB Hamiltonian. The formulas are also different. |
|
I know, but you think they could not be unified at all? Most if not all energy expressions from xTB did fit nicely in the provided framework of DFTB. Why should it be different for the multipoles? |
|
@awvwgk I don't think they could be unified as one. The difficulty of having multiples with DFTB is that it requires some additional integral evaluations. These integrals can be handled quite easily in the xTB framework, but not in the case of DFTB. I had to find a different way to make it work with DFTB. In addition, we used different kernel functions. I might be wrong. @aradi might have a better justification. |
|
I absolutely agree with @awvwgk that we should try to integrate things and should not have parallel infrastructures, if not necessary. I still have to understand, whether and how this is possible. @v2quan89 Let's first rebase the code to be compatible with current main. Then we can think about refactoring. (Thanks for the tests!) |
|
@v2quan89 the next step is now probably to start moving your changes to the newer code structure. How familar are you with the curent main branch code? There have been several additions since the 2020 version. |
|
@bhourahine I have started to move my changes to the current version. The current main code was restructured significantly. It gives me a hard time and might take longer time than I thought. In any case, I am working on it and will update the CHANGELOG and the manual as you suggested. |
|
@vnquanvuong See https://github.com/bhourahine/dftbplus/tree/mdftb_rebase for a rebased version on top of the current main code. I had to also make some changes to get it to compile with various different machines. |
|
@bhourahine Thank you very much for the help! I appreciate it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1325 +/- ##
==========================================
+ Coverage 70.86% 70.91% +0.04%
==========================================
Files 231 232 +1
Lines 44203 44948 +745
==========================================
+ Hits 31325 31875 +550
- Misses 12878 13073 +195 ☔ View full report in Codecov by Sentry. |
This version of the multipole expansion is based on DFTB+ 2019 and may require significant modification before merging with the main branch.