Move density build to solver object #262
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #262 +/- ##
==========================================
- Coverage 70.65% 70.63% -0.03%
==========================================
Files 159 160 +1
Lines 20058 20073 +15
Branches 7133 7138 +5
==========================================
+ Hits 14172 14178 +6
- Misses 2267 2271 +4
- Partials 3619 3624 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/tblite/context/type.F90
Outdated
| @@ -44,9 +46,11 @@ module tblite_context_type | |||
| !> Optional logger to be used for writing messages | |||
| class(context_logger), allocatable :: io | |||
| !> Optional factory for creating electronic solvers | |||
| class(context_solver), allocatable :: solver | |||
| class(context_solver), allocatable :: ctxsolver | |||
There was a problem hiding this comment.
Can we keep the original name? The ctx prefix hete does not seem necessary
There was a problem hiding this comment.
then I would need to rename the solver 4 lines down from this line. I wanted to have the actual solver as part of the context since this object usually persists in implementations.
There was a problem hiding this comment.
I think the context_solver is already flexible enough to allow keeping persistent memory or implementation details between runs. I would like to keep the current logic of instantiating a new solver in every singlepoint from the context_solver. You can use a pointer in the created solver which references the persistent context memory for example.
There was a problem hiding this comment.
Okay, I see your point, I will transfer the setup logic to the context solver.
Can you elaborate on why you think having this overhead in creating a solver every time makes sense?
|
in order to run the whole CI I added some fixes regarding the newest meson version giving errors. |
I would prefer a separate PR. Otherwise, we have to wait for this PR to be merged. |
|
@awvwgk can we move forward with this or do you want more changes? |
In order to prepare for solvers which are not based on diagonalization, I propose as done before by @awvwgk to move all the functions relevant to building the density matrix to the solver type object, this allows a later overwrite by a solver object that uses DMP methods for example.
Another functionality introduced by this PR is that the solver might not be deleted at the end of an SCF cycle if a flag within the context is changed. The reason for that is that for applications such as CREST or geometry optimizations in general, we would want to reuse the solver instance as to not pay the setup time too often. This is especially helpful in the context of GPU based methods as it allows to reuse buffers allocated on the GPU in latter SCF cycles.
Relating to #71.