Opened 5 years ago

Closed 5 years ago

#1378 closed Bug (fixed)

Fix bug in return code handling in cpl_oasis3.F90

Reported by: frrh Owned by: nemo
Priority: low Milestone: Unscheduled
Component: OCE Version: release-3.6
Severity: Keywords:
Cc:

Description

cpl_oasis3.F90 contains a bug which can lead to confusion when debugging coupled models using OASIS3/OASIS3-MCT etc.

cpl_prism_rcv obtains the return code variable "kinfo" from the call to prism_get_proto and, when ln_ctl is set, it intends to write this value out, along with various other pertinent values.

However, the "kinfo" value actually written is not the value returned from the coupler because it gets overwritten with a local, NEMO-specific value "OASIS_Rcv" (or "OASIS_idle") between the put call and the write statement. This is not what is intended (see cpl_prism_snd for analogy) and is not helpful for debugging coupled runs where one requires to know the real OASIS return code.

The simplest solution seems to be to move kinfo = OASIS_Rcv after the WRITE statements.

However, a clearer solution would be to employ completely different variable names for the two things. to make clear the fact that the value returned by prism_get_proto is not the same as the value returned by cpl_prism_rcv.

Commit History (2)

ChangesetAuthorTimeChangeLog
4806frrh2014-10-03T12:55:34+02:00

#1378 Apply simplest solution to ensure that return codes from oasis put operations are reported correctly without being overwritten by local return code values required by sbccpl.

Note: This version does not address the issue of best practice where the use of kinfo for two different purposes in the same routine should be disambiguated.

4805frrh2014-10-03T12:49:16+02:00

#1378 Apply simplest solution to ensure that return codes from oasis put operations are reported correctly without being overwritten by local return code values required by sbccpl.

Note: This version does not address the issue of best practice where the use of kinfo for two different purposes in th same routine should be disambiguated.

Change History (5)

comment:1 Changed 5 years ago by frrh

I propose to apply changes to vn3.4 stable branch and head of trunk if there are no objections.

comment:2 Changed 5 years ago by smasson

You can do the simplest solution.
However, if you are debugging, I suggest you switch ln_ctl = true and you will get the print. no?

comment:3 Changed 5 years ago by frrh

Yes, it's true that the existing line:

IF ( ln_ctl ) WRITE(numout,*) "llaction, kinfo, kstep, ivarid: " , llaction, kinfo, kstep, srcv(kid)%nid(jc)

will intercept and write the correct value, but this makes things even more confusing
to (the expanding army of) users who don't have intimate knowledge of NEMO
because they would need to know that a line which looks like:

llaction, kinfo, kstep, ivarid:

contains the correct information, while the more official looking lines
from the subsequent block of WRITE statements which explicitly claim to be reporting
the return code as

'prism_get_proto: info ', kinfo

is wrong.
This has actually caused significant problems for someone coupling a wave
model to an existing atmosphere-ocean set up. They looked at the NEMO
output and concluded that OASIS was returning an undefined status
for a put operation, resulting in them looking for bugs in the OASIS source code.

I'm happy to do the fix either way if you prefer the simple one.


comment:4 Changed 5 years ago by frrh

Applied simplest solution to head of trunk at revision [4806]

Applied simplest solution to stable vn3.4 branch at revision [4805]

comment:5 Changed 5 years ago by frrh

  • Resolution set to fixed
  • Status changed from new to closed

No issues reported following application of fixes.

Closing ticket, though ideally, long term we would still wish to address disambiguation issues.

Note: See TracTickets for help on using tickets.