mjhoptics / ray-optics

geometric ray tracing for optical systems
BSD 3-Clause "New" or "Revised" License
263 stars 54 forks source link

DecenterData.tform_before_surf() can return None as first member of tuple but code doesn't check for None #62

Closed dibyendumajumdar closed 2 years ago

dibyendumajumdar commented 2 years ago

I don't know if this is real issue but it makes the type checker unhappy as the tuple member 0 can be None but when this is referenced in the code there is no check.

Example in raytrace.py

rt, t = tfrm_from_before
b4_pt, b4_dir = rt.dot(before_pt - t), rt.dot(before_dir)

But above rt can be None.

mjhoptics commented 2 years ago

You have a point. I made a specific decision to write the lowest level code assuming the inputs (at least the input structures) are correct. This is to avoid having to test everything against a possible None or other invalid value. Another reason is adding validation tests is an optimization to handle "faulty" data. The code is written for the most general case; if None were a valid entry, a simpler calculation can be done, maybe with better speed? But that's an optimization and can be put off until someone shows its faster. Stated another way, this routine requires all of the inputs to be complete; if they aren't then the results aren't guaranteed. If the inputs cause a ray failure, this is not an input structure problem and try/except statements are used to guard against these kind of data dependent errors. There are currently only a few places where these transform tuples are computed, see the sequence and transform modules. trace_raw() is the primary consumer of the path iterator. I'll add to the doc there the assumption about error free input and also mention the requirement in the path() method in SequentialModel.