DROPLET and PARTICLE_CLASS, TARGETS and POINTERS

[gforney]

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 crogsch on 2009-02-17 14:58:40

[gforney]

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 issue reported on code.google.com by crogsch on 2009-02-17 15:08:53

[gforney]

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 issue reported on code.google.com by crogsch on 2009-02-17 15:14:54

[gforney]

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 issue reported on code.google.com by mcgratta on 2009-02-17 15:17:30

[gforney]

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 issue reported on code.google.com by drjfloyd on 2009-02-17 15:27:19

[gforney]

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 issue reported on code.google.com by crogsch on 2009-02-17 15:40:48

[gforney]

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

Original issue reported on code.google.com by crogsch on 2009-02-17 15:43:07

[gforney]

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 issue reported on code.google.com by mcgratta on 2009-02-17 16:19:47

[gforney]

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 issue reported on code.google.com by mcgratta on 2009-03-13 14:35:24

[gforney]

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 issue reported on code.google.com by crogsch on 2009-03-14 21:53:06

[gforney]

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 issue reported on code.google.com by mcgratta on 2009-03-15 15:40:39

[gforney]

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 issue reported on code.google.com by tkorhon1 on 2009-03-16 12:30:44

[gforney]

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 issue reported on code.google.com by tkorhon1 on 2009-04-21 12:39:09

[gforney]

(No text was entered with this change)

Original issue reported on code.google.com by mcgratta on 2009-11-23 22:26:14