Opened 7 years ago

Closed 7 years ago

#1222 closed Bug (fixed)

Issues with fldread at 3.6

Reported by: rfurner Owned by: nemo
Priority: highest Milestone:
Component: OCE Version: release-3.6
Severity: Keywords:
Cc:

Description

When running the AMM12 with the 3.6 merge code line 196-197 has a floating point zero divide. It seems to be coming from sd(jf)%nreclast which should be set as a non zero value earlier in the code…

Commit History (0)

(No commits)

Attachments (1)

fldread.F90 (84.0 KB) - added by rfurner 7 years ago.
proposed fix to fld_read

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by rfurner

It seems that in some cases (the fluxes in AMM12 for example) sd(jf)%nreclast is used in fld_read (line 197) ahead of it being defined, and this at the time of use it is zero.

sdjf%nreclast is defined at the end of fld_clopn, however, fld_clopn is called within fld_init ONLY if sdjf%ln_tint is true. The calls to fld_clopn within fld_read occur after line 197, where nreclast is used. I think this might be what is causing my troubles, but not sure…

Changed 7 years ago by rfurner

proposed fix to fld_read

comment:2 Changed 7 years ago by rfurner

It seems the issue is in fld_init. fld_clopn is only called if using interpolation, however in the non-interpolation case a fld_clopn is still needed for the first time step of a new year/month/week/day (depending on the file type). The attached fld_read proposes a fix which seems to solve things.

It would be really useful if someone with a good understanding of the fld routines could confirm this is sensible!!

thanks!!

comment:3 Changed 7 years ago by smasson

I think we could simply add a default definition of nreclast in fld_fill. something like

         sdf(jf)%nreclast        = -1

Am I wrong?

comment:4 Changed 7 years ago by rfurner

I'm not massively confident…but…

I think when we begin, and when we move to a new file (by moving to the next year if yearly files, next week if weekly files etc) then we need to have a call to fld_clopn, regardless of the state of ln_tint. This means the above, whilst it may stop things crashing, wont really fix that issue. so I think calling fld_clopn when we move to a new file is a better solution (and I think is more in line with the way things were done in 3.4, but not sure why it changed, hence my lack of confidence on this one).

I'm happy to be persuaded otherwise, but the issue isn't just nreclast being missing data, but that its being used as index of the last data point in the file, despite not having yet been set to that value…setting it to a default value which still doesn't represent the value of the index of the last data point wont fix this properly…I think.

Am I wrong?

comment:5 Changed 7 years ago by smasson

  • fld_init is called only at the first time step, see the use of ll_firstcall. It is used only to make a first call of fld_rec (define associated variables) and read the before data when doing time interpolation.
  • the call to fld_clopn done at line 208 is the one that open new file (if needed) in all cases. The one in fld_init (line 389) is only needed to read the before data when you do a time interpolation. The one in line 246 is use to read the after data if written in a new file. So we are indeed calling fld_clopn when moving to a new file if we do temporal interpolation or not.
  • the test in line 196-197 is used to avoid a tricky and odd case described lines 191 and 192. This case cannot happen at the first time step (so there is the .NOT. ll_firstcall) however at this stage, you are right, nreclast is not defined if you don't do any time interpolation. if the default definition of nreclast is 0, you get the divide by 0 error because of the modulo line 196. At the first time step, we don't care of the initial value of nreclast as the case will be false because ll_firstcall = .true., but nreclast must be defined and not equal to 0. Therefore I propose -1

Do you agree?

comment:6 Changed 7 years ago by rfurner

ahhh, that makes a lot more sense. yes, you are definitely right, sorry!

I can stick this fix in first thing tomorrow.

Thanks!

comment:7 Changed 7 years ago by rfurner

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.