[Liblas-devel] Throwing out_of_range exceptions on read

[Mateusz Loskot]

Howard Butler wrote:
> On Apr 27, 2008, at 5:52 PM, Mateusz Loskot wrote:
>> Howard Butler wrote:
>>>
>>> Mateusz,
>>> Mark in Florida has some 1.1 LAS files that have points with Scan
>>> Angle Ranks that are outside the -90/90 range that the specification
>>> says.
>>
>> Hobu,
>>
>> Seems, Mark's file does not follow the LAS specification.
>> Is that possible?
> 
> Yes, I agree.  I think the file is out of spec.

OK.

>> If yes, then I think it would be worth to know what's the generator of
>> such file and provide appropriate warning/comment in libLAS FAQ.
>
> 
> The offending software writes 1.1 files with the following:
>   Project ID/GUID:           '00000000-0000-0000-0000-000000000776'
>   System Identifier:         'NIIRS10'
>   Generating Software:       'LIDAR1 tiled'

I found that NIIRS10 means LIDAR 1 CuePac software by GeoCue.

>>> I just commented out the check for now:
>>> http://liblas.org/browser/trunk/src/laspoint.cpp#L113
>>
>> OK
>>
>>> This begs the question, what do we do for reading invalid input?
>>
>> I would prefer to keep validating it.
> 
> I don't know that we can keep validating it *where* we validate 
> currently -- in the getter/setter methods of LASPoint.

Yes, sounds reasonable.

> Maybe we could have something like
> -- ReadNextPoint(bool validate)
> -- if (validate) {LASPoint::validate();}

Good idea.

> and have LASPoint::validate toss an exception.  This way we can do it on 
> write as well.  I'm not sure out_of_range is the best exception to throw 
> either.

It probably isn't, but I'd not worry to much.
I'm going to introduce some dedicated exceptions this week/weekend,
as planned here http://liblas.org/ticket/12

> Some things that might be good candidates for validate():
>  - Invalid time values
>  - Scan angle rank
>  - flight edge not being 1 or 0
>  - invalid classifcation values
>  - invalid scan direction

Seems like we could base on rules described in the specification:
if the spec defines valid value/range of a data, then let's check it.

>>> Scan angle doesn't really mean too much, so for this case, following
>>> the spec exactly isn't going to be a deal, especially if there are
>>> other writers out there who aren't following. But for other items,
>>> especially the flags attributes, this is a big deal.
>>
>> Your suggestion sounds reasonable to me. However, I'd like to propose 
>> some modifications (below) :-)
>>
>>> Any ideas on a reasonable tact to take?  Liberal in what we read and
>>> strict in what we write?
>>
>> 1. The ASPRS LAS format is suppose to be a data exchange format,
>> so we always must [sic] write in strict mode. IMHO, if LAS writers are 
>> very  relaxed, then interoperability - a very principle of ASPRS LAS 
>> format - will be on paper only.
> 
> Such is the fallacy of all standards :)

I imagine, place the fallacy of my belief the World is ideal ;-)

> I think we should have LASWriter do validate() on write,
> and we should not allow folks to circumvent this with a flag.

Good idea. I support it.

> It adds a few checks and might cost some 
> raw write performance, but it is just pushing the existing test that 
> lives in the LASPoint methods around so it isn't a net gain or loss in 
> performance.

Yes.

>> 2. I'd vote for libLAS reading in semi-relaxed mode by default, what 
>> actually you are suggesting above, but in strict mode on request (a 
>> configuration setting, a program parameter, etc.).
> 
> For read operations, I think we should have a flag that allows you to 
> use relaxed reading, but we should default to validate() on read as 
> well, IMO.

Good idea.

> For translation operations like las2las, this might mean 
> throwing out points that aren't to spec, because write() isn't going to 
> allow it.  I would have las2las provide summary in that instance.

Yes, plus some note in the manual should do the job well.

> The only way to get people to write files in spec are to tell them 
> they're writing it out of spec :)

I think so too.

> Also, I would venture a guess that 
> libLAS is the first LAS format library out there to really care about 
> validating the parts of the base spec we implement.

I believe it scores :-)

>> 3. While reading LAS file in semi-relaxed, the library should always 
>> print a warning to stdout.
> 
> I have changed all of the utilities to emit text output to stderr 
> because stdin/stdout can be used for regular file i/o for some of them.  


Right. All diagnostics should go to stderr. BTW, when using C++ streams,
std::cerr and std::clog are best candidates. Then it's easy to redirect 
these streams to file, for instance.

> IMO, the library shouldn't print error messages anywhere, and it should 
> notify the application that there's an error or warning and let it 
> decide what to do.  I already have stuff in the C API's error handling 
> to be able to deal with this, I think.

OK, works for me.

> Anyone else with thoughts on this issue, please chime in.  I don't want 
> to box folks in because a lot of times you don't control who's giving 
> you LAS files to deal with.  On the other hand, I don't want to 
> perpetuate crappy broken data either...

Personally, I like your idea.
If no new concepts will reach the list, I suggest to implement what 
we've discussed already and push it to users. Nothing is fixed in 
concrete, so we can improved the solution in future.

Greetings
-- 
Mateusz Loskot
http://mateusz.loskot.net