Feat/netcdf status warning - #361
Conversation
The NetCDF 'status' variable in sfincs_map.nc / sfincs_his.nc previously
mirrored `error` (0 = clean, 1 = instability stop). A completed run with a
NaN or out-of-range water-level boundary still reported status=0, because:
- the boundary sanity check used zs_bnd < -99 .or. zs_bnd > 990, which is
both-false for a NaN and so silently missed it; and
- the only path that sets error=1 (dtchk < dtmin) is defeated by a NaN
(NaN < dtmin is false), so the instability guard never fires.
Changes:
- sfincs_data: new non-fatal `warning` flag (does NOT affect ierr/exit code).
- sfincs_lib: initialise warning=0 next to error=0.
- sfincs_boundaries: detect low/high/NaN via .not.(x >= -99 .and. x <= 990),
which catches NaN too (a NaN fails every ordered comparison); set warning=1.
- sfincs_ncoutput: new run_status() -> 0 (clean) / 1 (error, wins) / 2
(completed with warning); write it to the map & his 'status' variable and
document 0/1/2 in the long_name.
A fatal error still takes precedence over a warning. Deliberately avoids
ieee_arithmetic (ieee_is_nan pulls a UCRT frexp that clashes with Intel
libmmd at link time, LNK2005, with oneAPI 2025.0.4).
Builds clean (Release|x64, ifx 2025.0.4): sfincs_lib 0/0, sfincs.exe links.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Commit 9ef639f ("Shorter messages") was meant to trim comments, but it also reverted the logic line itself back to zs_bnd < -99.0 .or. zs_bnd > 990.0 which is both-false for a NaN and therefore misses it. That silently undid the fix from b539c75, so the branch tip could only flag finite out-of-range values. Restored .not. (zs_bnd >= -99.0 .and. zs_bnd <= 990.0) which catches low, high and NaN in one test, with a two-line comment instead of the original eight. Verified on D:/software/SFINCS/20260824_issue_NaN/benchmarkNAN (all-NaN bzs, 129 boundary points): the warning is logged and map+his status = 2. Control run with the same bzs and nan->0.0 gives status = 0. Release|x64 builds 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014nT6uVicosj59tcRLqo9vj
Leynse
left a comment
Leynse
left a comment
There was a problem hiding this comment.
Hi Kees,
I like it!
Only question- should we include this warning = 1 status for more written warnings we raise?
There are many other boundary conditions forcings with checks...
And also other warnings
Maybe not include everywhere where we raise a written warning message, but maybe including some more important once like:
- WARNING! Found wavemaker point along quadtree refinement boundary, this is not recommended! The simulation will continue.'
- WARNING! ', nrwovl, ' cells are covered by both wavemaker polyline files !
- ETC
OR, separate between a minor warning and severe warning??
We can discuss (/with Maarten).
ALSO, question - if we call 'stop_sfincs' for when e.g. an input file is missing.
Does this than actually also end up in netcdf status as error currently?
If not, we should add that.
What do you think?
|
Hi Tim, thanks! Yes, same flag for all of them. One code (2 = finished, but read the log), no minor/severe split. I wouldn't hook it to every line that says "Warning" though. Missing tref, no EPSG, old binary format: those run perfectly. I would only set it when SFINCS silently changed or dropped something the user asked for, so NaN or out-of-range forcing, clipped time series, skipped points, obs points outside the domain, and your wavemaker cases. Not for a filled-in default or a deprecation notice. To keep it manageable I'll add a write_warning() in sfincs_log that logs the line and prints "finished with N warning(s)" at the end. stop_sfincs never reaches netcdf. It's a plain Fortran stop and nearly every call happens while reading input Let's go over it with Maarten when he is back from holiday. Cheers, Kees |
2 participants
Minor new implementations to allow for proper warning and status =2 which means the simulation didnt run as expected