Opened 14 years ago

Closed 4 months ago

Last modified 4 months ago

#19 closed defect (fixed)

Non-fatal errors are apparently fatal

Reported by: David Loeffler Owned by: Olly Betts
Priority: minor Milestone: 1.2.7
Component: cavern Version: 1.1.16
Keywords: Cc:

Description

Running cavern on the top-level all.svx file in the "sample data" tarball produces a bunch of errors about malformatted dates:

david@rockhopper:~/survex/loser> ~/bin/cavern all
Survex 1.1.16
Copyright � 1990-2011 Olly Betts
In file included from surface/allnr41.svx:22,
                from surf.svx:26,
                from all.svx:20:
surface/sonnausn.svx:8: Invalid day of the month
In file included from surface/allnr41.svx:118,
                from surf.svx:26,
                from all.svx:20:
surface/228ausn.svx:10: Invalid day of the month
In file included from caves/031/031.svx:15,
                from caves/caves.svx:7,
                from all.svx:27:
caves/031/elchalt.svx:8: Invalid day of the month
In file included from caves/040/040arge.svx:7,
                from caves/040/040.svx:21,
                from caves/caves.svx:9,
                from all.svx:27:
caves/040/arge/elefant.svx:8: Invalid day of the month
In file included from caves/040/040arge.svx:19,
                from caves/040/040.svx:21,
                from caves/caves.svx:9,
                from all.svx:27:
caves/040/arge/e6.svx:8: Invalid day of the month
In file included from caves/040/040arge.svx:25,
                from caves/040/040.svx:21,
                from caves/caves.svx:9,
                from all.svx:27:
caves/040/arge/niete.svx:9: Invalid day of the month
In file included from caves/041/041.svx:77,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/mark.svx:8: Invalid day of the month
In file included from caves/041/041.svx:78,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/seiteng.svx:8: Invalid day of the month
In file included from caves/041/041.svx:87,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/bigchamb.svx:8: Invalid day of the month
In file included from caves/041/041.svx:92,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/germanrt.svx:8: Invalid day of the month
In file included from caves/041/041.svx:93,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/e41b.svx:8: Invalid day of the month
In file included from caves/041/041.svx:95,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/steinsch.svx:8: Invalid day of the month
In file included from caves/041/041.svx:96,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/megalo.svx:8: Invalid day of the month
In file included from caves/041/041.svx:97,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/wdr.svx:8: Invalid day of the month
In file included from caves/041/041.svx:98,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/lampntot.svx:8: Invalid day of the month
In file included from caves/041/041.svx:101,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/glueck.svx:8: Invalid day of the month
In file included from caves/041/041.svx:103,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/din.svx:8: Invalid day of the month
In file included from caves/041/041.svx:106,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/edelmean.svx:8: Invalid month
In file included from caves/041/041.svx:107,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/rondos.svx:8: Invalid month
In file included from caves/041/041.svx:111,
                from caves/caves.svx:10,
                from all.svx:27:
caves/041/arge/alice.svx:8: Invalid day of the month
In file included from caves/078/078.svx:90,
                from caves/caves.svx:11,
                from all.svx:27:
caves/078/fetzig.svx:8: Invalid day of the month
In file included from caves/078/078.svx:92,
                from caves/caves.svx:11,
                from all.svx:27:
caves/078/stuttg.svx:8: Invalid day of the month
In file included from caves/115/115.svx:23,
                from caves/caves.svx:15,
                from all.svx:27:
caves/115/nebukad.svx:8: Invalid day of the month
In file included from caves/144/144.svx:25,
                from caves/caves.svx:29,
                from all.svx:27:
caves/144/arge/144ein.svx:9: Invalid day of the month
In file included from caves/144/144.svx:26,
                from caves/caves.svx:29,
                from all.svx:27:
caves/144/arge/144horiz.svx:8: Invalid day of the month

Removing trailing traverses...

Concatenating traverses between nodes...

Simplifying network...

Solving 2 simultaneous equations

Solving to find x coordinates...

Solving 2 simultaneous equations

Solving to find x coordinates...

Solving one equation

Solving to find x coordinates...

Solving 101 simultaneous equations

Solving to find x coordinates...

Solving 7 simultaneous equations

Solving to find x coordinates...

Solving 28 simultaneous equations

Solving to find x coordinates...

Solving 58 simultaneous equations

Solving to find x coordinates...

Solving one equation

Solving to find x coordinates...

Solving 2 simultaneous equations

Solving to find x coordinates...

Calculating network...

Calculating traverses between nodes...

Calculating trailing traverses...

Calculating statistics...

Survey contains 11442 survey stations, joined by 11492 legs.
There are 266 loops.
Survey has 216 connected components.
Total length of survey legs = 66026.52m (66028.33m adjusted)
Total plan length of survey legs = 49308.43m
Total vertical length of survey legs = 30114.66m
Vertical range = 1085.31m (from sternloch.ent at 1850.00m to
115.115rest.orgasm.1 at 764.69m)
North-South range = 4570.79m (from lunge.dreamfudge at 85498.95m to
115.115rest.orgasm.17 at 80928.17m)
East-West range = 3321.34m (from raetsel.4 at 38281.23m to 183.lower.1
at 34959.89m)
 161 0-nodes.
 926 1-nodes.
9194 2-nodes.
1014 3-nodes.
 122 4-nodes.
 16 5-nodes.
  5 6-nodes.
  2 7-nodes.
  2 8-nodes.
CPU time used  0.41s
Done.
There were 0 warning(s) and 25 non-fatal error(s) - no output files produced.

Although it describes the date errors as "non-fatal", there's no way to make it output anything! It would be nice to have a switch that forced it to output a 3d file anyway; otherwise in what sense are the errors "non-fatal"?

Change History (16)

comment:1 Changed 14 years ago by Olly Betts

Status: newassigned

Repeating my reply from the list to keep this together:

The terminology here is that a "fatal error" is one which means that the program simply can't continue (so something like a disk error, or running out of memory). An non-fatal error is only non-fatal in that we don't have to bail out right away - there's nothing which completely prevents further progress.

(I'm not sure this is actually a useful way to think about it - this has raised similar questions before (i.e. "why doesn't it produce files if the errors are non-fatal?"). Probably just dropping "non-fatal" here would actually be clearer.)

The main reason we keep going is to show further errors, so the user can see everything which needs fixing at once, avoiding the extremely irritating syndrome where they get to fix just one error for each reprocessing.

I can see a case for a bad date being a warning - the real mitigating factor is that cavern used to be less fussy here, so there are datasets which used to process but no longer do.

I've dropped the "non-fatal" in r3592.

Still pondering what to do about whether this should be a warning instead. Being fussy for users without legacy data seems good overall, so simply relaxing it to a warning isn't great. It could be conditional on *require 1.1.7 or later (without that it would be a warning, with it an error), but it doesn't seem totally ideal to link the concepts of "I'm using a new feature" and "this data is free of legacy issues".

comment:2 Changed 14 years ago by David Loeffler

I'd still argue in favour of an "--ignore-errors-if-at-all-possible" command-line option.

comment:3 Changed 14 years ago by Olly Betts

That would work, but it would be nice to have an approach which works well when cavern isn't run from the command line - opening a .svx file in aven is now working pretty well, and I think it's likely to be how the majority of users will process files in the future, but it is clumsy if it has to stop to ask for options to use before processing.

comment:4 Changed 14 years ago by Olly Betts

Milestone: 1.1.17
Version: 1.1.16

comment:5 Changed 13 years ago by Olly Betts

Milestone: 1.2.01.2.1

comment:6 Changed 13 years ago by Olly Betts

Milestone: 1.2.11.2.2

comment:7 Changed 13 years ago by Olly Betts

Milestone: 1.2.21.2.3

comment:8 Changed 13 years ago by Olly Betts

Milestone: 1.2.31.2.4

comment:9 Changed 13 years ago by Olly Betts

Milestone: 1.2.41.2.5

comment:10 Changed 13 years ago by Olly Betts

Milestone: 1.2.51.2.6

1.2.5 was released last month...

comment:11 Changed 13 years ago by Olly Betts

Milestone: 1.2.61.2.7

comment:12 Changed 13 years ago by Wookey

To allow the option to work for gui users as well as on the command line have a settings file (/etc/survex/options ~/.config/survex/options) in which default options could go. GUI users would access this via a 'settings' menu somewhere where they could tick 'ignore processing errors if possible' which just adds that option to the conf file. We already have something like this for setting default printing options IIRC?

If survex ever gets turned into a proper library then this goes in the API.

A bit fiddly but it should work, and useful beyond just this one case.

comment:13 Changed 13 years ago by Olly Betts

This is really something which "belongs" to the dataset though, rather than to the installation.

If I send you a dataset which works for me because I've set this, then it won't work for you unless you have too. That's clearly unhelpful.

I think the best options are:

  • Add a new command (or possibly add a new flag to *flags) which allows this to be controlled. That also would allow a dataset to be converted to have parseable dates gradually rather than only being able to enforce this or not for the whole dataset (or worse for a whole installation). This is also handy if you connect two caves where the dates are good in one but not in the other.
  • Make this a warning rather than an error.

comment:14 Changed 12 years ago by Olly Betts

Milestone: 1.2.7

I've made this case a warning in commit ce8f81, so it should be in the next release (1.2.7).

That addresses the immediate issue (so unsetting the milestone), though it would be good to have a scheme for addressing this class of issue where we really want to make cavern fussier for new data when there is existing data we don't want to break. Perhaps "warning" is that scheme, which would be least work. Or we could easily add another class of diagnostic which is sort of between warning and error for use in such cases.

comment:15 Changed 4 months ago by Olly Betts

Milestone: 1.2.7
Resolution: fixed
Status: assignedclosed

The issue with *date in existing data from before we started parsing its argument triggering errors was addressed in 1.2.7 by demoting that error to a warning.

The confusing "non-fatal" vs "fatal" error distinction is no longer made in output (functions like fatalerror() still exist in the code, but they just report an "error" and then exit. Looks like that was in 1.2.0.

it would be good to have a scheme for addressing this class of issue where we really want to make cavern fussier for new data when there is existing data we don't want to break. Perhaps "warning" is that scheme, which would be least work. Or we could easily add another class of diagnostic which is sort of between warning and error for use in such cases.

Overall I think the extra complication is not justified, and we should just be mindful of not breaking reasonable existing usage when deciding whether a new diagnostic is a warning or an error. In some cases that existing usage is broken and we were just lacking a check, so sometimes a new Survex version may give an error on existing data, but such cases should be very much the exception and when there's a reasonable fallback we should issue a warning and do that. Typically that will probably be doing what existing versions do, as in the case of *date where if the format is wrong we issue a warning and ignore the *date command).

Therefore closing.

Last edited 4 months ago by Olly Betts (previous) (diff)

comment:16 Changed 4 months ago by Olly Betts

A related point - in 1.4.10 unconnected surveys are now a warning rather than an error. If there are other cases which are currently an error but it might be useful to users to continue, please highlight them and we can look into changing them to warnings.

Note: See TracTickets for help on using tickets.