DROPLET and PARTICLE_CLASS, TARGETS and POINTERS

[GoogleCodeExporter]

Please complete the following lines...

SOURCE Code, last revision

Describe details of the issue below:
During doing some programming with FDS, I found, that e.g. in the PART.f90
file, pointers DR for DROPLETS and PC for PARTICLE_CLASSES are not
initialized, like other pointers. Have a look at the short listing from
part.f90. You can see, that FVXS,... are initialized as POINTERS, but DR
and PC not, thus DR and PC are non-stack Variables. An initialization will
fix this problem. I added the missing lines and two comments in the snipplet

SUBROUTINE PARTICLE_MOMENTUM_TRANSFER

! Add droplet momentum as a force term in momentum equation

USE COMP_FUNCTIONS, ONLY : SECOND
REAL(EB), POINTER, DIMENSION(:,:,:) :: FVXS,FVYS,FVZS
REAL(EB) :: FLUXMIN,XI,YJ,ZK
INTEGER :: II,JJ,KK,IIX,JJY,KKZ,I,J,K,IC,IW

-> THIS is not in the file, but should be
TYPE(DROPLET_TYPE), POINTER :: DR
TYPE(PARTICLE_CLASS_TYPE), POINTER :: PC
-> END

FVXS  => WORK1
FVYS  => WORK2
FVZS  => WORK3
FVXS  = 0._EB
FVYS  = 0._EB
FVZS  = 0._EB

SUM_MOMENTUM_LOOP: DO I=1,NLP
   DR=>DROPLET(I) !!!!!!Here DR is not a stack variable
   PC=>PARTICLE_CLASS(DR%CLASS) !!!!!!!Here PC is not a stack variable
   IF (DR%IOR/=0)   CYCLE SUM_MOMENTUM_LOOP
   IF (PC%MASSLESS) CYCLE SUM_MOMENTUM_LOOP
   XI  = CELLSI(FLOOR((DR%X-XS)*RDXINT))
   YJ  = CELLSJ(FLOOR((DR%Y-YS)*RDYINT))
   ...
This "not initialized" can be found in PART, DUMP and VEGE (for DR=> and
PC=>). In READ.f90 PC is correct initialized.

Furthermore, sometimes the types are initialized as TARGETS, sometimes not
(like PARTICLE_CLASS = TARGET in type.f90, DROPLET is not initialized as
TARGET in MESH.f90. I think, the initialization should be done always in
the same way.

Kind regards,
Christian

Original issue reported on code.google.com by crog...@gmail.com on 17 Feb 2009 at 2:58

[GoogleCodeExporter]

Just a short note about the target. If variables initialized with the TARGET
attribute, the compiler may be able to produce a faster or more efficient code, 
if
they are used with a pointer. So, all possible TARGETS should be initialized as 
a
TARGET. In the case of WORK1,... or DROPLET I think this should be done.

Original comment by crog...@gmail.com on 17 Feb 2009 at 3:08

[GoogleCodeExporter]

I found in the literature the following statement about pointer initialization:

REAL, POINTER :: ptr(:) =>NULL()

Initialization should be done with  =>NULL(), because a pointer with no 
initialization,
means its association status is undefined. This is valid for Fortran 95/2003 
only
(not Fortran90)

Original comment by crog...@gmail.com on 17 Feb 2009 at 3:14

[GoogleCodeExporter]

Christian, thanks. Both DR and PC are declared at the top of the module PART. 
Do you 
suggest that it is better to declare them within each subroutine? I have never 
understood completely the difference between declaring variables for an entire 
module, as opposed to declaring in the individual subroutines.

Original comment by mcgra...@gmail.com on 17 Feb 2009 at 3:17

• Changed state: MoreInfo

[GoogleCodeExporter]

Defined at the top of a module means the variables are available to all 
subroutines
in the module and to any other module or subroutine that includes the module 
with a
USE statement.  Defined in a subroutine, the variables are only available to 
that
subroutine.

Original comment by drjfloyd on 17 Feb 2009 at 3:27

[GoogleCodeExporter]

I found this "error" with the initialization of DR and PC by using INTEL
ThreadChecker. With the actual configuration (PC and DR declared at the top of 
the
module) the ThreadChecker writes a statement, that DR and PC are non-stack 
variables.
I think (but I'm not sure) this might be a compiler "problem". Based on the 
kind of
optimization it could be easier for the compiler to produce a fast code, if all
variables and pointers are located in the subroutine. If they are declared only 
one
time at the top of the module, the whole module must be "checked". But this is 
my
personal opinion.
I personally recommend to write the  POINTER statement in each subroutine, 
because I
think this will reduce compiler problems (and of course, it will remove the
ThreadChecker statement ;-) )

Original comment by crog...@gmail.com on 17 Feb 2009 at 3:40

[GoogleCodeExporter]

Jason is right. But based on the error-message of the ThreadChecker I think the 
Intel
Compiler compiles it in two different ways.

Original comment by crog...@gmail.com on 17 Feb 2009 at 3:43

[GoogleCodeExporter]

I think I know what you are describing, in very general terms. A few years ago, 
we 
were getting strange compiler and run-time errors that we did not understand 
until 
we decided to declare variables locally, rather than globally within a given 
module. 
Stack, heap, and all that sort of thing. I remember distributing globally 
defined 
variables into the individual subroutines. That seemed to help. I think we 
should do 
this in part.f90 too. We should do whatever we can to help the compiler better 
optimize, even if we don't understand completely why.

Original comment by mcgra...@gmail.com on 17 Feb 2009 at 4:19

[GoogleCodeExporter]

I just committed a new part.f90 in which DR and PC are defined locally. Sorry 
this 
took so long. It was not difficult to do. If this problem comes up again, I 
usually 
just comment out the globally-defined variable and then compile in debug mode. 
The 
compiler will tell you where you need to define the variable locally. The only 
thing 
to watch out for is if a variable is used in multiple subroutines within the 
module 
such that it's value needs to be stored globally.

Original comment by mcgra...@gmail.com on 13 Mar 2009 at 2:35

• Changed state: Fixed

[GoogleCodeExporter]

Thanks. If this problem or other problems occurs I can fix it, thus I have 
access to 
the code ;-) The aim is to produce a "warning-free" code, thus this edits give 
more 
stability and reduces debugging time with other problems which may occure. If I 
get 
a warning from Intel ThreadChecker in the parallelization process, I will fix 
it (e. 
g. see edits in turb.f90). This does not not mean that anybody has written 
wrong 
code lines, this is only a help for producing code which is more compiler 
independent. If the Intel ThreadChecker produces a warning it is not possible 
to 
guarantee that the code works correct in a parallel OpenMP version.

Original comment by crog...@gmail.com on 14 Mar 2009 at 9:53

[GoogleCodeExporter]

Christian, thanks. Send an email to 

kevin.mcgrattan@nist.gov
jfloyd@haifire.com
simo.hostikka@vtt.fi
randall.mcdermott@nist.gov

if you make a change to a file that is not strictly an addition of an 
OpenMP "comment". One of us can then check to make sure that the change is OK. 
You 
can send the email outside of the Issue Tracker or Discussion Group. 

Original comment by mcgra...@gmail.com on 15 Mar 2009 at 3:40

[GoogleCodeExporter]

Hi Christian,

the file evac.f90 uses quite much HR-pointers, which are not defined
at the beginning of the current subprogram/function. And there are quite
many cases, where HR is used in some subprogram and the HR ==> xxx is done
in the calling procedure, so you should not yet try to "OpenMP" evac.90.
Well, this means that ieva.f90 is also not so important for "OpenMP".

I will try to put "TARGET" keywords somewhere and also "=>NULL()". And
I will also try to put the pointer definitions in the subroutines, if
these are not needed as module variables. But this takes some time.

Ciao,
TimoK

Original comment by tkorh...@gmail.com on 16 Mar 2009 at 12:30

[GoogleCodeExporter]

Now evac.f90 has the "TARGET" keywords, and "=>NULL()" also. And the pointers 
are
now defined in the subroutines, not at the beginning of the module.

TimoK

Original comment by tkorh...@gmail.com on 21 Apr 2009 at 12:39

[GoogleCodeExporter]

Original comment by mcgra...@gmail.com on 23 Nov 2009 at 10:26

• Changed state: Closed