Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove BGJ spaghetti code #364

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jeffhammond
Copy link
Collaborator

BGJ added a bunch of spaghetti that serves no purpose. This is the easiest and most important to remove.

@jeffhammond jeffhammond marked this pull request as draft May 4, 2021 19:53
Copy link
Collaborator

@edoapra edoapra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this change is going to work since it is the one that initializes bgj_rtdb (QA tests might work with this PR if rtdb=0)
It might need to replaced by a call in nwchem.F to bgj_init(rtdb)

       subroutine bgj_init(rtdb)
       implicit none
#include "bgj_common.fh"
      integer rtdb
      bgj_rtdb = rtdb
      return
      end

@jeffhammond
Copy link
Collaborator Author

Yeah, it's definitely not working yet. I am in the process of ripping out a lot more stuff, and adding rtdb as an argument to many routines, to eliminate the BGJ spaghetti.

@jeffhammond jeffhammond marked this pull request as ready for review May 10, 2021 20:50
@jeffhammond
Copy link
Collaborator Author

@edoapra I ran the test suite and the results seem favorable but I know we don't have perfect test coverage. I was unable to get IBM XLF to -qextchk this the way I remember it doing in the past. Anyways, it might be worth inspection now.

Unfortunately, because of the way ga_iter_lsolve involves Fortran functions, I have to pass the rtdb argument via evil methods, but it is localized and theoretically can be fixed if we change everything (ROHF, UHF, ROKS, UKS, etc) the same way.

@jeffhammond
Copy link
Collaborator Author

@edoapra i think this is ready for review now. i ran the full test suite on it a while ago and it was fine.

@edoapra
Copy link
Collaborator

edoapra commented Feb 11, 2022

Some of the properties tests are still failing.
One reason could be that you have not changed the interface to some of the shell_fock_build variants (e.g. shell_fock_build_cam)

@jeffhammond
Copy link
Collaborator Author

ok i will look again. thanks. the test suite was running slowly.

@edoapra
Copy link
Collaborator

edoapra commented Feb 11, 2022

ok i will look again. thanks. the test suite was running slowly.

A couple of tests are pretty slow right now because they do convert 64_to_32 (against the full list of lapack routine) the whole source with the idea of detecting lapack calls.

H2O (RHF) runs correctly through the following:

task scf optimize
task scf freq
task dft optimize
task dft freq

this is hardly sufficient but identified a lot of issues.

the IBM XLF compiler will identify a bunch of argument mismatch issues once i use it.

Signed-off-by: Jeff Hammond <[email protected]>
@edoapra edoapra force-pushed the master branch 2 times, most recently from 790a699 to 4524b64 Compare April 13, 2022 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants