Code review hack! - #1
Conversation
… UHF subroutines (now just duplicates of existing subroutines) for future development
…mulations directly in master
…limiting steps are due to asymmetrised integrals, need to make slices of them next.
…ct CCSD energies.
…specification of number of error matrices, documentation
ajwt
left a comment
ajwt
left a comment
There was a problem hiding this comment.
Generally very nicely written code! I'd be interested in comparisons of performance between different scheduling or vectorization regimes.
|
|
||
| file (GLOB_RECURSE sources src/*.f90 src/*.F90) | ||
| add_executable(els.x ${sources}) | ||
| target_link_libraries (els.x /opt/OpenBLAS/lib/libopenblas.a -lpthread) |
There was a problem hiding this comment.
Wouldn't hurt to make this more configurable - having to have a dirty CMakeLists.txt hanging around because you don't have access to /opt isn't great practice.
| ! ovlp%W holds the eigenvalues s_i of S | ||
| associate(diag=>ovlp%W) | ||
| do i = 1, size(ortmat, dim=1) | ||
| ortmat(i,i) = 1/sqrt(diag(i)) |
There was a problem hiding this comment.
For large bases this might end up blowing up owing to near linear dependence. Might be worth at least checking the value of diag(i) isn't too small, and throwing an error if it is.
| end do | ||
|
|
||
| if (.not. conv) then | ||
| write(iunit, '(1X, A)') 'Convergence not reached, please increase maxiter.' |
There was a problem hiding this comment.
Is this worth throwing an error or perhaps returning something to note you haven't converged?
| end if | ||
|
|
||
| call deallocate_diis_t(diis) | ||
| deallocate(st%density_old) |
There was a problem hiding this comment.
Doing this here seems a bit out of place. Perhaps have a routine in system.F which does the allocation and deallocation of the bits of state_t?
| integer :: n_errmat = 6 | ||
| integer :: n_active = 0 | ||
| integer :: iter = 0 | ||
| real(p), allocatable :: e(:,:,:) |
There was a problem hiding this comment.
definitely document what these are - also helpful to loosely know what the order of the subscripts is eg:
!F(norb,norb,n_max_diis_steps)
| end subroutine update_cc_energy | ||
|
|
||
| subroutine do_ccsd_t_spinorb(sys, int_store) | ||
| ! Spinorbital formuation of CCSD(T) |
There was a problem hiding this comment.
Reference for the maths you're implementing?
|
@ajwt I've made some significant changes to the code in the past 5 days, namely I've added the following functionalities:
A head-scratcher for me is that the spin-free CCSD gives the wrong energies (and consequently I can't test the (T)/[T] methods). I've been at it for a week now, and completely coded the equations up from scratch again, and it's still the same, wrong energy. The extent of the error is about 0.5 mHa at eqm geometry for water/def2-svp, and almost 10 mHa at even moderately stretched 1.6 A, before completely diverging at 1.8 A. I've ruled out DIIS being the culprit as turning it off doesn't help. In my repo, under p.s. the water/def2-svp data for r_OH = {1.00..0.20..1.80} can also be found in my repo under |
2 participants
No description provided.