[Octopus-devel] [Octopus-notify] svn commit: r8394 - trunk/src/hamiltonian by jrfsousa

[José Rui Faustino Sousa]

Hi Xavier,

On Mon, 2011-10-17 at 16:17 -0400, Xavier Andrade wrote: 
> Hi Jose,
> 
> What is the problem with the OpenMP directives? This should be fixed
> in 4.0.x (but only the problem with the directives).
> 

Well I think I was a bit too strong in my wording.

The private clause of one of the "do" loops listed the inner "forall"
index as private, I think that it is most likely wrong, and it is my
understanding that "forall" constructs must be parallelized  using
"parallel workshare", as any matrix assignment, and not "parallel do",
but that only counts for the outer loop.

> BTW, what is the reason to change the loop structure?
> 

I think that the loop structure did not change. The inner "forall" is
useless it is just standing in for a simple matrix assignment, the code
generated should be the same, the outer "do" loop can be rewritten as a
"forall" construct and that should raise no problems and some
temporaries were removed. I would also replace the the "1:n" syntax with
just ":" but I am not sure if that can be safely done.

> I think the
> original version was faster than the new one and this is one of the
> parts of Octopus that is critical for performance.
> 

I would think that a good compiler should generate faster code this way.
But of course sometimes in practice things are not as good as they
should be. Rewriting "do" loops as matrix assignments, "forall"
constructs are a kind of matrix assignment, should yield more
opportunities for a good compiler to generate better optimized code.

But then again the code in question is quite complicated and it is very
likely that most compilers will not produce optimal code and use
temporaries for evaluation of the array intermediate results and that
will kill performance.

And all this means that I probably jumped the gun and that you are
right, the best way to rewrite those loops would be as some nice and
thigh "do" loops. I will do that ASAP.

Best regards,
José Rui


> Xavier
> 
> On Mon, Oct 17, 2011 at 10:37 AM,  <octopus-svn at tddft.org> wrote:
> >      Author: jrfsousa
> >        Date: Mon Oct 17 15:37:14 2011
> > New Revision: 8394
> >   Changeset: http://www.tddft.org/trac/octopus/changeset/8394
> >
> > Modified:
> >   trunk/src/hamiltonian/hamiltonian_base_inc.F90
> >
> > Log:
> > Bug fix, wrong OpenMP directives, rewrote some loops
> >
> > _______________________________________________
> > Octopus-notify mailing list
> > Octopus-notify at tddft.org
> > http://www.tddft.org/mailman/listinfo/octopus-notify
> >
> _______________________________________________
> Octopus-devel mailing list
> Octopus-devel at tddft.org
> http://www.tddft.org/mailman/listinfo/octopus-devel
>