OrderN / CONQUEST-release

Full public release of large scale and linear scaling DFT code CONQUEST
http://www.order-n.org/
MIT License
96 stars 25 forks source link

Tidy up calls for restarting from existing density matrix #249

Closed davidbowler closed 8 months ago

davidbowler commented 8 months ago

There were various inconsistencies and issues with loading an existing density matrix which have been fixed; this should now be correct and efficient

davidbowler commented 8 months ago

As part of this PR I would like to remove the part of initialisation_module.f90 that sets the L matrix using initial_L. These lines are only ever called if we set the flags to create the initial charge density from the K matrix but do not load the L matrix (which would be a very peculiar thing to do). The initial L matrix is set the the inverse overlap which is about the best approximation that could be made, but is still terrible. @tsuyoshi38 what do you think?

https://github.com/OrderN/CONQUEST-release/blob/6bf8f4a8c20fd4fa8f1c7baeb8a6b1f23a6d2408/src/initialisation_module.f90#L1270-L1281

tsuyoshi38 commented 8 months ago

I am sorry for my late reply. I agree that the changes in initialisation_module.f90 is okay.
(I have noticed that this issue was originally raised by me long time ago. Sumimasen..)

For the deleted part in minimise.f90, I do not clearly remember why we had them. Th effects of the control variables reset_L and restart_DM are tricky, but I wonder I needed them to optimize the DM for some unstable cases. Could you give me a little more time ?

tsuyoshi38 commented 8 months ago

To be honest, I did not have time to remember what I did to control DMM steps using these flags. But I agree that the code should be clearer and the present changes are okay.

davidbowler commented 8 months ago

I have removed initial_L and tidied up the calls so that reset_L is now set in get_E_and_F solely. Small changes to ensure that we don't try to build charge density from DM without loading DM.