[MITgcm-devel] REAL4_IS_SLOW is broken

[Jean-Michel Campin]

Hi Martin,

I think I remember: I was lazy !
And at that time, we had only:
_rl version used only in monitor(s) (only for _rl variables)
_rs version used only to print grid stats (in ini_grid.F & ini_cori.F)
So I did not create the 2 others S/R (which would not
have been called at that time).
Thing starting to get worse when surface forcing monitor was
added (_rl/_rs was not the only problem, mask are also not right
with more that 1 tile / node, but it bypass the call in this case).

If we want to keep only 2 S/R, we will need, in ini_grid.F and ini_cori.F, 
to define 1 local array (with bi,bj) and to fill it with 1., and then to 
use it as mask,hFac & area in the mon_printstats_rs calls.

What I would propose instead is:
a) to rename mon_printstats_rs.F -> mon_printgstats_rs.F
   &  rename mon_stats_rs.F      -> mon_gstats_rs.F
  ( grid-statistics as opposed to "domain-statistics" which accounts
  for mask, area, hfac ...)
b) change the calls in ini_grid.F
c) from the _rl versions (mon_printstats_rl.F & mon_stats_rl.F)
   create 2 similar _rs S/R.

Does this make sense ?
Cheers,
Jean-Michel

On Sun, Oct 26, 2008 at 05:13:02PM +0200, Martin Losch wrote:
> Hi Jean-Michel,
>
> in order to make the code not too ugly (i.e., avoid the appearance of  
> the CPP-flag REAL4_IS_SLOW in monitor.F that I have introduced  
> yesterday), I'd like to change mon_printstats_rs.F and mon_stats_rs.F to 
> make them alike the corresponding mon_*_rl.F-variants. Can you remember 
> any reason, why (6years ago) you only modified the mon_*_rl.F files and 
> not the short real ones? Was it simply unnecessary? Can I do it now?
>
> Martin
>
> On 25 Oct 2008, at 20:59, Martin Losch wrote:
>
>> Hi again,
>>
>> the model runs with my modifications and seems to pass the testreport 
>> (not quite done yet, but I don't see why it should fail for the last 
>> few experiments) on eddy.csail.mit.edu, however,
>>
>> both mnc (for grid variables in write_grid.F) and diagnostics,  
>> regardless of mnc, for the _RS variables (e.g. surface forcing) is  
>> broken if READ4_IS_SLOW is undefined. As far as I can see, the  
>> diagnostics package is not set up for the RL/RS and always assumes  
>> real*8 variables. I can fix write_grid.F and also the monitor of the 
>> surface fields (requires mon_printstats_rl to be replaced by rs- 
>> version, not a big deal), but the diagnostics is hopeless I guess.
>>
>> Should I go ahead and do these changes? If so, what can be done about 
>> the diagnostics? Issue a warning, if the critical variables are active 
>> (in data.diagnostics)? I don't know how to do that.
>>
>> Even if the diagnostics stuff cannot be sorted out, I still think that 
>> fixing the REAL4_IS_REAL flag  for the rest of the code is a good 
>> thing, so I would opt to do the changes. What do you think?
>>
>> Martin
>>
>> On 25 Oct 2008, at 16:59, Martin Losch wrote:
>>
>>> Hi Chris,
>>>
>>> I think I found the problem: in CPP_EEMACROS.h the expansion of the 
>>> _EXCH_*RS/4 macros is independent of REAL4_IS_SLOW, that is they 
>>> always expand into RL like this:
>>> #define _EXCH_XY_RS(a,b) CALL EXCH_XY_RL ( a, b )
>>> #define _EXCH_XYZ_RS(a,b) CALL EXCH_XYZ_RL ( a, b )
>>> #define _EXCH_XY_R4(a,b) CALL EXCH_XY_RL ( a, b )
>>> #define _EXCH_XYZ_R4(a,b) CALL EXCH_XYZ_RL ( a, b )
>>>
>>> I fixed this by putting the into ifdefs and expanding them to _RS in 
>>> the cae of REAL4_IS_SLOW. That did the trick. I will certainly  
>>> check, if this breaks the testreports, but in case it doesn't break 
>>> the tests, can I just check this in (I am asking, because I don't 
>>> feel too comfortable futzing around in this area; maybe you or 
>>> anybody else want to have a look at my modifications frist?)
>>>
>>> Martin
>>>
>>> PS. Should we have a test that checks this functionality, or is it  
>>> not so important?
>>>
>>>
>>> On 25 Oct 2008, at 07:36, Martin Losch wrote:
>>>
>>>> Hi Chris,
>>>>
>>>> after our telephone conversation yesterday, I quickly tried #undef 
>>>> REAL4_IS_SLOW, to see whether this has any effect on the quadcore 
>>>> performance. It compiles without any problems, but the code stops 
>>>> at the temperature == 0 test in ini_theta.F, some THETA values are 
>>>> zero. Actually many values are zero, not only temperature, because 
>>>> after exchange there are stripes of zeros along the tile edges in 
>>>> all grid fields (most of them are now real*4, but theta is still 
>>>> real*8).
>>>>
>>>> Now, I did expect this to work right away, but since you and  
>>>> Dimitris have already gone through using the _RS -> real*4  
>>>> business, maybe you remember what you did exacty, or maybe this is 
>>>> documented somewhere?
>>>>
>>>> Our domain here at Weizmann is a cartesian grid, so I am NOT using 
>>>> exch2, as you might have with Dimitris. Do you have any idea, 
>>>> what's going on? If it's not so complicated to do I am willing to 
>>>> fix this in the code (never good to have feature that does not 
>>>> work, right?), otherwise we should include a comment in  
>>>> CPP_EEOPTIONS.h that this flag REAL4_IS_SLOW always needs to be  
>>>> defined
>>>>
>>>> Martin
>>>> _______________________________________________
>>>> MITgcm-devel mailing list
>>>> MITgcm-devel at mitgcm.org
>>>> http://mitgcm.org/mailman/listinfo/mitgcm-devel
>>>
>>> _______________________________________________
>>> MITgcm-devel mailing list
>>> MITgcm-devel at mitgcm.org
>>> http://mitgcm.org/mailman/listinfo/mitgcm-devel
>>
>> _______________________________________________
>> MITgcm-devel mailing list
>> MITgcm-devel at mitgcm.org
>> http://mitgcm.org/mailman/listinfo/mitgcm-devel
>
> _______________________________________________
> MITgcm-devel mailing list
> MITgcm-devel at mitgcm.org
> http://mitgcm.org/mailman/listinfo/mitgcm-devel