search

molpro / iterative-solver

MIT License
0 stars 3 forks source link

Extend Fortran/C interface to specify hermiticity - [merged] #290

Closed pjknowles closed 2 years ago

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 29, 2020, 09:26

Merges non-hermitian -> master

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 29, 2020, 10:28

added 9 commits

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 29, 2020, 15:32

added 1 commit

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 29, 2020, 15:33

marked this merge request as draft

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 29, 2020, 15:38

Now done as far as bringing back the class variable to signal whether hermitian (I don't understand why it was removed....). We need to complete this to have a chance of Molpro regression tests working again (TDDFT as formulated is non-hermitian).

@mxs301 can you please help with the actual implementation? I am completely lost as to how the class is now structured. There is so much virtualisation, so many subsidiary classes, that I am lost; is there any chance to get this understandable by an outsider? How to pass m_hermitian down to influence the subspace solution?

Consideration should probably be given to the code sharing between linear eigensystem and linear equations. They are almost identical, it's just really the subspace solver that is different. I placed m_hermitian in IterativeSolver but maybe this is not the best, as it is not relevant for non-linear problems.

pjknowles commented 3 years ago

In GitLab by @mxs301 on Nov 30, 2020, 01:13

Yes, it's probably better to move it to linear solvers.

You can have a look at LinearEigensystem to get an idea of the structure (https://molpro.gitlab.io/linearalgebra/latest/classmolpro_1_1linalg_1_1itsolv_1_1LinearEigensystem.html). There are two levels of interfaces, one common to all solvers and one specific to eigensystem, and two levels of implementation respectively. It's just a linear chain of inheritance, I can't think of a way to make it simpler without jumbling everything into one god class.

The main work done by IterativeSolverTemplate is construction and solution of the subspace problem. The low level details are moved to XSpace and SubspaceSolver. IterativeSolverTemplate uses them through interface classes, because implementation can vary depending on the algorithm. Their instances must be pass through the only constructor, https://molpro.gitlab.io/linearalgebra/latest/classmolpro_1_1linalg_1_1itsolv_1_1IterativeSolverTemplate.html#a132eb84dab97daf27a4f66969221bce2.

In terms of adding new functionality one has to consider the intent at every level. For example, adding specification of hermiticity. Is it common to all solver? No, it's specific to linear problems so we should add it to ILinearEigensystem and ILinearEquations interfaces. If it is not common to all solvers, than IterativeSolvertemplate does not need to be modified. LinearEigensystem provides instances of XSpace and SubspaceSolver, it also implements end_iteration. Out of those, only XSpace and SubspaceSolver are affected by hermiticity, so that flag has to be propagated. Hermiticity is algorithm specific, so the interface for those classes does not need to change, only their implementation.

This line of thinking had to be done in the old iterative solver as well, when it was all just one class. The interfaces and subdivision into classes just forces everyone to apply it.

pjknowles commented 3 years ago

In GitLab by @mxs301 on Nov 30, 2020, 02:04

added 14 commits

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @mxs301 on Nov 30, 2020, 02:21

added 3 commits

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @KnowlesPJ on Nov 30, 2020, 18:00

added 1 commit

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @mxs301 on Nov 30, 2020, 21:46

added 2 commits

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @mxs301 on Nov 30, 2020, 21:48

@KnowlesPJ Hermiticity should be fixed. For now I just copy the H matrix from row-wise to column-wise format. If this becomes a problem, we can make things more efficient by using column-wise storage in the Matrix class.

This can probably be merged into master.

pjknowles commented 3 years ago

In GitLab by @mxs301 on Dec 1, 2020, 24:23

added 1 commit

Compare with previous version

pjknowles commented 3 years ago

In GitLab by @mxs301 on Dec 1, 2020, 24:53

marked this merge request as ready

pjknowles commented 3 years ago

In GitLab by @mxs301 on Dec 1, 2020, 24:54

resolved all threads

pjknowles commented 3 years ago

In GitLab by @mxs301 on Dec 1, 2020, 24:54

mentioned in commit 5d44749a859657f30b50319d58855bee30181604