search

CABLE-LSM / CABLE

Home to the CABLE land surface model and its documentation
https://cable.readthedocs.io/en/latest/
Other
12 stars 6 forks source link

431 add routine for more informative namelist errors #438

Closed Whyborn closed 2 weeks ago

Whyborn commented 3 weeks ago

CABLE

Thank you for submitting a pull request to the CABLE Project.

Description

Add a routine to handle namelist errors, that returns a much more informative error message than the baseline namelist error message. Utilises the new IOMSG specifier and the routine detailed here.

Fixes #(431)

Edit: Turns out that this namelist checking routine relies on undefined behaviour. The problems I was seeing was the result of a "regression" in Intel Fortran 2023, see this thread. The comment from Steve Lionel in the accepted answer is important:

The standard says, "If an error condition occurs during execution of an input/output statement, the position of the file becomes indeterminate."

This means that the so-called regression was not really a regression, just a change in already undefined behaviour. This use of BACKSPACE to return the incriminating line relies on undefined behaviour that may change at any time. Based on this, I decided against including this part of the routine, and just throwing back the IOMSG returned by the READ call.

Type of change

Checklist

Please add a reviewer when ready for review.

Whyborn commented 3 weeks ago

For some reason, I can only get it to replicate the desired behaviour on Gadi by including two BACKSPACE(nmlUnit) calls, rather than one. I built a dummy test case on my own machine and it behaved as desired with a single BACKSPACE.

Gadi ifx: 2024.2.1 Local ifx: 2024.0.2

The current commit contains a single BACKSPACE call, which is how I think it should work. Trying it on the test case I'm using for BIOS on Gadi, it writes the line after the problematic line to the STDERR stream.

Whyborn commented 3 weeks ago

What's even stranger is that I copied the routine into a little standalone routine (see here) and compiled and ran it on Gadi, and it worked as desired with a single backspace. The namelist I used with this gist is

test.nml

&testnml
    var1 = 1
    abadvar = thiswillfail
    var2 = 2
/

So it doesn't seem like a compiler difference. A bit stumped? Why does the version in CABLE need 2 BACKSPACE calls to get the incriminating line?

SeanBryan51 commented 3 weeks ago

That's weird. Could it be a compiler flag issue?

Whyborn commented 3 weeks ago

I feel like it must be related to that, but I tried with most of the flags that come with the DEBUG build I've been using and couldn't replicate the behaviour. I'll put it on the back-burner for now.

Whyborn commented 2 weeks ago

I've added made the changes to all the namelist read calls I found in the CABLE code (ignored stuff in old/ and UM/). We could extend similar changes to all instances of READ in the code, but I think that might be too far out of scope.