[Octopus-devel] [Octopus-notify] svn commit: r4355 - trunk/src/states by lorenzen

[Xavier Andrade]

Hi,

Actually I think that having explicit boundaries in vectorial expressions 
should be mandatory in Octopus. At least for expressions with two or more 
arrays, this causes a lot of troubles because sometimes we can't be sure 
of the size of the array we are using or someone might want to change the 
size of the array and because different compilers consider the implicit 
boundaries differently (for example gfortran considers the maximum size, 
while ifort the smallest).

For example, in the final fix of Florian, someone afterwards may want to 
reuse tmp, innocently increasing the size again to np_part, introducing 
the bug again.

Cheers,

Xavier

On Mon, 7 Jul 2008, Tobias Burnus wrote:

> octopus-notify at tddft.org wrote:
>>       Author: lorenzen
>>         Date: Fri Jul  4 09:16:22 2008
>> New Revision: 4355
>>    Changeset: http://www.tddft.org/trac/octopus/changeset/4355
>> Modified:
>>    trunk/src/states/restart.F90
>> Log:
>> This fixes the failure of open_systems/03-extended_eigenstate.test in
>> i386_gfortran_test:#356. It cirumvents a bug in GFortran.
>>
>
> I would argue that this was a bug in Octopus and not in gfortran:
>
>
> -  st%zphi((k-1)*m_lead%np+1:k*m_lead%np, idim, jst, k_index) = tmp
> + ! Do not write this inner loop as vector assignment. GFortran
> + ! has a problem with that.
> + lb = (k-1)*m_lead%np+1
> + do ip = lb, k*m_lead%np
> +   st%zphi(ip, idim, jst, k_index) = tmp(ip-lb+1)
> + end do
>
> lb  is 1  and  k*m_lead%np  is two. Thus: LHS has a size of two.
> However, tmp and thus the RHS has a size of 12.
> Thus LHS and RHS have a size mismatch.
>
>
> In the explicit loop, one iterates over the LHS and thus it works.
> Similarly should work:
>
> -  st%zphi((k-1)*m_lead%np+1:k*m_lead%np, idim, jst, k_index) = tmp
> +  st%zphi((k-1)*m_lead%np+1:k*m_lead%np, idim, jst, k_index) =
> tmp(1:(k*m_lead%np)-(k-1)*m_lead%np+1)+1)
>
>
> * * *
>
> Seemingly, this size mismatch this is not tested with g95 and ifort - at
> least for the following program "ifort -check all" and "g95
> -fbounds-check" produce no error:
>
> integer,allocatable :: a(:)
> integer :: b(1)
> allocate(a(12))
> b(:) = a
> end
>
> (gfortran and NAG f95 find the problem; NAG: "Rank 1 of A has extent 12
> instead of 1".)
>
> * * *
>
> I therefore suggest the following patch:
>
> Index: ../octopus-trunk/src/states/restart.F90
> ===================================================================
> --- ../octopus-trunk/src/states/restart.F90     (Revision 4361)
> +++ ../octopus-trunk/src/states/restart.F90
> @@ -697,8 +697,6 @@
>       ! It only works in this compact form because the
> transport-direction (x) is
>       ! the index running slowest.
>       do k = 1,
> nint(sb%lsize(TRANS_DIR)/gr%sb%lead_unit_cell(LEFT)%lsize(TRANS_DIR))
> -        ! Do not write this inner loop as vector assignment. GFortran
> -        ! has a problem with that.
>         lb = (k-1)*m_lead%np+1
>         do ip = lb, k*m_lead%np
>           st%zphi(ip, idim, jst, k_index) = tmp(ip-lb+1)
>
>
> An alternative would be to find out why tmp is too big and whether it is
> supposed to be too big.
>
> Tobias
> _______________________________________________
> Octopus-devel mailing list
> Octopus-devel at tddft.org
> http://www.tddft.org/mailman/listinfo/octopus-devel
>