yambo-code / yambo

This is the official GPL repository of the yambo code
http://www.yambo-code.eu/
GNU General Public License v2.0
100 stars 39 forks source link

Refact fast collisions #105

Closed attacc closed 4 months ago

attacc commented 5 months ago

The bug was due to the test-suite and not to the code, I fixed it.

In the while I found another addition bug and I fixed that one too, this is the reason for this pull request.... tests are runnnig with random parallelization, I wait the end of tests

attacc commented 5 months ago

Tests are fine, they do not fail anymore. https://media.yambo-code.eu/robots/refact_fast-collisions/attacc-lavoro_refact_fast-collisions_1_error.php (there are fails in SC on my PC but there are not due to my changes... they are present also in master etc..)

andreamarini commented 5 months ago

Ciao. How do I check the code if it comes from an external repo?

I mean. If I want to do a diff with the reference master (tech-master in this case) is there any other way to clone the personal repo?

attacc commented 5 months ago

This is easy, if you click on "Files changed" github shows you all changes respect to the branch where the pull is proposed, in this case tech-master https://github.com/yambo-code/yambo/pull/105/files

andreamarini commented 5 months ago

I know. But I want to compile, test, check.... not just see the diffs.

attacc commented 5 months ago

Ok in this case I do like this:

GPL_FOLDER='yambo_gpl'
BRANCH='refact_fast-collisions'
TEST_BRANCH='tech-master'

git clone git@github.com:attacc/yambo.git $GPL_FOLDER
cd $GPL_FOLDER
git checkout $BRANCH
git pull

cd $TEST_FOLDER
git checkout $TEST_BRANCH
git pull
./driver.pl -c
nice -n 19 ./driver.pl -gpl -input -flow validate -b -branch $BRANCH >> output_validate

ciao ciao Claudio

andreamarini commented 5 months ago

Yeah.

I understand this pain is needed for external developers! But one day you have to explain me why I need to do the same pain with a senior developer that is part of the team.

BTW. The robots system DOESN'T work in the case of branch coming from a fork. So the automatic robots can check only branches of the devel/gpl repo.

So it looks to me that this decision of using ONLY forks is just a way to increase the complexity.

Boh.

attacc commented 5 months ago

Ciao Andrea

for me it is fine to make a branch in the gpl repository... no problem, if you want I transfer my change there

Ciao claudio

andreamarini commented 5 months ago

git clone git@github.com:attacc/yambo.git $GPL_FOLDER cd $GPL_FOLDER git checkout $BRANCH git pull

Cla, sorry if I appeared a little bit exhausted... this new way of developing in Yambo for me that is doing everything in the devel repo is a little bit of a pain.

I need still to figure out how to extract code to be proposed for the gpl from my branches in the devel. And in the meantime I see that the gpl is continuously developing. So I am late. But as I am the only one developing in the develop none is taken care of the devel->GPL procedures.

In addition also the test-suite looks to be only my responsability. With the php robot page that is still not working...

So any additional pain is a big pain.

If you could create a branch in the gpl repo it would be great! In the meantime I am coding the procedure to clone an external repo and bla bla

image

attacc commented 5 months ago

No problem, I will create it

sangallidavide commented 5 months ago

Fine with me. Just I see lines commented in the file OSCLL_eval. If we are thinking about restoring them in the future, we could add some tag before the comment, such as: ! TO_BE_FIXED Otherwise we can remove them.

sangallidavide commented 5 months ago

For external branches, this is the way I use them, in case you like it

I'm in my local version of yambo

$pwd
/data/Codes/yambo/yambo-gpl/master
$git remote -v
origin  git@github.com:yambo-code/yambo.git (fetch)
origin  git@github.com:yambo-code/yambo.git (push)

I add the repo of the developer

$git remote add attacc git@github.com:attacc/yambo.git
$git fetch attac

After that, for checking out the branch locally to test it, is the same procedure for branches in the gpl repo and in the attac repo, just set origin/attac in the --track option as below

$git worktree add -b refact_fast-collisions  /data/Codes/yambo/yambo-gpl/branches/refact_fast-collisions --track attacc/refact_fast-collisions

In short, my local version has all the branches, and the repo is just another "folder" as the "tech/maintenance" etc, which points to the develper. Also, doing so the remote yambo-gpl report remains clean.

attacc commented 5 months ago

There are commented lines because I tried to reduce memory loading WF for each pair k and k+q, but then I get a segmentation fault in WF_load when I finish the loop and I want to start a new calculation. I'm not able to solve it at the moment so I commented the lines, but I will come back to it in the future

attacc commented 5 months ago

Math in the PDF and the tutorial on wiki

andreamarini commented 5 months ago

Thanks so much Cla. Math is clear. ASAP I will check the tutorial and run the code.

Quick question: is the use of LSEX limited to NL?

More along this line. Why didn't you use the standard SEX but using NG=1? I mean. Isn't all the code doing exactly what SEX is doing just using the scalar part of W? To account for local-fields one can still calculate the screened interaction if NG>1 but then read only the head.

attacc commented 5 months ago

The reason is that the collisions require a huge amount of memory and disk space... we cannot effort them for large systems. We need something simpler. Our idea is to do not include local-field effects but only the LSEX. It is reasonable approximation for excitons in homogeneous systems.

attacc commented 5 months ago

No, in principle LSEX is not limited to NL, for this reason I left the subroutine OSCLL_compose_nl.F the use the density matrix to build it, instead of the valence bands

attacc commented 5 months ago

The branch is online

sangallidavide commented 4 months ago

I tested it, and finally the NL tests are not failing in parallel. For me it can be merged.

attacc commented 4 months ago

Also tests on my machine are fine: https://media.yambo-code.eu/robots/refact_fast-collisions/attacc-lavoro.php

attacc commented 4 months ago

I merge it all tests are fine... documentation is online etc...

andreamarini commented 4 months ago

I have added the lsex.pdf as reference in the wiki tutorial