grasp icon indicating copy to clipboard operation
grasp copied to clipboard

DDotMPI and DGEMVMPI implementation

Open CYChenFudan opened this issue 5 years ago • 10 comments

MPI versions for the blas subroutines ddot and dgemv are presented. They are included in lib/mpif90/mpiu.f90, then are called by dvdson module.

CYChenFudan avatar Jun 05 '20 11:06 CYChenFudan

Ok, I'm slowly getting up to speed after summer vacation. Sorry for the delay!

You have done many things here Chen, very nice! Just summarizing the changes you have made here, so it's easy for everyone to see:

Note: [output] means that the edit only affects output from the code to screen, and is therefore a less sensitive modification.

  1. src/tool: Makefile, add "chmod a+x $(GRASP)/bin/rsave" [already implemented]
  2. jj2lsj90: In jj2lsj_code.f90, lines 220 and 222 are changed to deal with both parities present. [rejected]
  3. rcsfzerofirst90: In RCSFzerofirst.f90, the number of CSFs in zero-order space are output. The informations could be used conveniently in RCSF and RCI calculations. [output, ok]
  4. rmcdhf90: In newco.f90, weighted average energy is output to monitor the energy convergence. [output, ok]
  5. rangular90_mpi: getinf.90 modified for iccut=1 [output, ok]
  6. rci90_mpi: setham_gg.f90, only myid=0 reports how far the calculation has proceeded. [output, ok]
  7. libdvd90, dvdson.f90: make lines 1119 and 1120 work for convergence monitor. The output messages are very useful for extremely large-scaled calculations. [output, ok]
  8. libdvd90, dvdson.f90: function TSTSEL is modified, so that dvdson performs at least TWO iteration calculations. Or, error results could be obtained in some cases (in O-like isoelectronic sequence?). [check, cff]
  9. The rscfgenerate bug that was discussed during the spring (issue #26) [already merged through separate PR]
  10. And finally the MPI versions of DDOT and DGEMV - leading to a parallell version of DVDSON

I need help with testing these changes out properly before we add them to master - any of the more senior members perhaps? @tspejo @gaigalas @cffischer or others?

No 1 (jj2lsj) [rejected], 4 (rangular) and 6-7 (dvdson) are the critical ones of the first group. Then we have the rcsfgenerate fix in no 8, and the larger update with new parallell versions of DDOT and DGEMV in No 9 which will require some testing.

@gaigalas - could you perhaps verify that update no 1 in jj2lsj on both parities is ok? [done] And maybe also the other updates you feel you can verify easily. @cffischer - could you have a look at update 7 perhaps?

jongrumer avatar Sep 22 '20 08:09 jongrumer

Dear Morten,

   Of course, please make the necessary changes.  We are somewhat busy in these days. But Ran and Kai, together with the students in our group could join the codes testing program. Please tell us if any modifications should be tested (Though the changes suggested have been extensively tested in F77-version). 

With best wishes, Chongyang

CYChenFudan avatar Sep 24 '20 08:09 CYChenFudan

@CYChenFudan Me and @mortenpi realized that you have submitted this pull request from your forked master branch of grasp. This is not ideal perhaps since any edits we will do now on the patch, will also directly affect your personal main version of the code.

For this time around to get this done as soon as possible, is it ok with you that we go ahead and do the necessary edits and deletes on your master branch? We will try to do as few edits/deletes as possible.

For the next time, first create a separate dev branch on your fork that you then submit a pull request from. We will probably squash-merge this PR, which will create a single commit on master that is different from the numerous ones that exist on your forked master branch. This means that from Git's point of view, the CompAS master and you master will diverge once we merge. So you will have to reset you master to the actual master from that on. For this reason, usually, pull requests are preferably made from separate branches to avoid that.

Oh, and try to reply on the github online chat instead of by email to make the thread less cluttered: here https://github.com/compas/grasp/pull/45

jongrumer avatar Sep 24 '20 09:09 jongrumer

@jongrumer @mortenpi It is OK that you go ahead and do the necessary edits and deletes on my master branch.

CYChenFudan avatar Sep 24 '20 13:09 CYChenFudan

@CYChenFudan Great, we'll get started with the stuff we actually can deal with now then, like sorting out the conflicting files. We need to wait for input from the other CompAS members on the larger changes like the one related to DVDSON.

jongrumer avatar Sep 24 '20 14:09 jongrumer

@gaigalas - could you perhaps verify that update no 1 in jj2lsj on both parities is ok? And maybe also the other updates you feel you can verify easily. This is not good idea for many reasons. So I am not planning to check what is not efficient.

gaigalas avatar Sep 30 '20 12:09 gaigalas

@gaigalas Ok great - I trust your judgement of course, so we shall remove the edits to jj2lsj (part 1 in the list above) from this patch. Let us know if you have opinions on any of the other edits. But I think we can handle them now, this was the most critical to get info on. Thanks a lot!

jongrumer avatar Sep 30 '20 14:09 jongrumer

OK. I will look other issue, too. But, there are the files *.f90 for 2, 3, 5 ... ?

gaigalas avatar Sep 30 '20 14:09 gaigalas

@gaigalas Great thanks GG 👍 I think I have most of them under control, I updated the list above now with "status comments", but if you could have a look at the edits to no 7 libdvd90, dvdson.f90: function TSTSEL and if possible also the MPI-libraries in no 9, that would be very helpful. My feeling is that we can merge 7 but have to wait with 9 until tested more.

You can see the changes to the files directly here on github quite easily by clicking on the "Files changed" tab up on the top of this page. This takes you here: https://github.com/compas/grasp/pull/45/files where you can scroll through the files. The files related to dvdson and mpi-blas are a bit down. Red lines are those that are removed, and green the new ones. If you have any direct comments on a certain line of code, you can press the blue "plus"-sign in the source code. This opens upp a message box where you can write your comment related to that particular line.

If you want to have a look at the files locally on your own computer, you have to download Chens version of the codes, either by pressing the green button "Code" --> "Download ZIP" at his page https://github.com/CYChenFudan/grasp or by cloning the repository directly to some folder at your computer with:

git clone https://github.com/CYChenFudan/grasp.git

Let me know if you have any further questions, and thanks again for the help!

jongrumer avatar Sep 30 '20 15:09 jongrumer

I've now split out the rcsfgenerate fix to a separate PR (#55) that I will merge now directly since this is the most crucial fix to get into master as soon as possible as far as I can tell.

I will edit my first summarizing comment accordingly.

jongrumer avatar Oct 06 '20 12:10 jongrumer