Feature, Code V .seq file IO #86
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements Code V .seq file format support for reading and writing geometric lens systems, completing the previously TODO-marked functionality. The implementation includes file parsing, surface object creation, and file generation for the Code V optical design format.
- Adds
read_lens_seq()method to parse Code V .seq files and create lens surface objects - Adds
write_lens_seq()method to export lens data to Code V .seq format - Removes the NotImplementedError and enables .seq file support in the main
read_lens()method
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| deeplens/geolens_pkg/io.py | Implements complete Code V .seq file I/O functionality with parsing logic, surface creation, and file writing; also includes code formatting improvements (whitespace cleanup) |
| deeplens/geolens.py | Removes NotImplementedError and enables .seq file reading by calling read_lens_seq() |
Comments suppressed due to low confidence (1)
deeplens/geolens_pkg/io.py:406
- Except block directly handles BaseException.
except:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| i += 1 | ||
|
|
||
| # Save the last surface | ||
| if current_surface: |
There was a problem hiding this comment.
Bare except clause catches all exceptions including SystemExit and KeyboardInterrupt. Specify the exception type(s) to catch, such as except (ValueError, IndexError):.
| if current_surface: | |
| except (IndexError, ValueError): |
|
|
||
| traceback.print_exc() | ||
|
|
||
| # Key: accumulate distance at the end of the loop |
There was a problem hiding this comment.
The traceback module is imported inside an exception handler. Move this import to the top of the file with other imports for better performance and readability.
| WTF 1.0 1.0 1.0 | ||
| VUY 0.0 0.0 0.0 | ||
| VLY 0.0 0.0 0.0 | ||
| DOR 1.15 1.05 |
There was a problem hiding this comment.
The attribute self.rfov is referenced but read_lens_seq() sets self.hfov instead (line 278). This will cause an AttributeError if a .seq file is read and then written. Change to self.hfov or ensure self.rfov is properly set.
| f" Image plane position: d_sensor={d:.4f}, r_sensor={self.r_sensor:.4f}" | ||
| ) | ||
| break |
There was a problem hiding this comment.
The magic number 18.0 as a default sensor radius is hardcoded. Consider defining this as a named constant at the class or module level (e.g., DEFAULT_SENSOR_RADIUS = 18.0) to improve maintainability and make the default value's meaning clear.
|
|
||
| sensor_str = f"SI 0.0 0.0\n" | ||
| sensor_str += f" CIR {self.r_sensor}\n" | ||
| lens_seq_str += sensor_str |
There was a problem hiding this comment.
Variable previous_material is not used.
| d_next = self.surfaces[i + 1].d - surf.d | ||
| else: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 7
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| # Spherical surface | ||
| s = Spheric(c=surf_c, r=surf_r, d=d, mat2=mat2_name) | ||
| s = Spheric(c=surf_c, r=surf_r, d=d, mat2=mat2) |
There was a problem hiding this comment.
Bug: ZEMAX Spherical Surface Loading Error
The Spheric constructor is passed mat2=mat2, but mat2 is undefined. The variable mat2_name was defined earlier (lines 71-77) and should be used instead. This causes a NameError when reading ZEMAX files with spherical surfaces.
| surf_param8, | ||
| ], | ||
| k=surf_conic, | ||
| mat2=mat2, |
There was a problem hiding this comment.
Bug: ZEMAX Aspheric: Constructor Variable Mismatch
The Aspheric constructor is passed mat2=mat2, but mat2 is undefined. The variable mat2_name was defined earlier (lines 71-77) and should be used instead. This causes a NameError when reading ZEMAX files with aspheric surfaces.
| print(f" Done! Created {len(self.surfaces)} objects") | ||
| print(f" d_sensor={self.d_sensor:.4f}") | ||
| print(f" r_sensor={self.r_sensor:.4f}") | ||
| print(f" hfov={self.hfov:.4f}°") |
There was a problem hiding this comment.
Bug: Print Fails on Conditional Attribute
The print statement accesses self.hfov which is only set if the .seq file contains a "YAN" line (line 274). If the file lacks this line, an AttributeError occurs. A default value or existence check is needed before printing.
|
|
||
| for i, surf in enumerate(self.surfaces): | ||
| if i < len(self.surfaces) - 1: | ||
| d_next = self.surfaces[i + 1].d - surf.d |
There was a problem hiding this comment.
Bug: Inconsistent Types Break Output Formatting
The d_next calculation returns a torch.Tensor when i < len(self.surfaces) - 1, but returns a float in the else branch (line 581). This inconsistency causes d_next to be a tensor in most iterations, which may produce incorrect string formatting when used in f-strings (lines 612, 633). Both branches should convert to float for consistency.
Feature provided by Wd
Note
Adds full CODE V .seq lens import/export and wires it into GeoLens file loading, including parsing/writing of surfaces, materials, aperture, and aspheric parameters.
deeplens/geolens_pkg/io.py):read_lens_seq()to parseEPD,YAN,SO/S/SI,CIR,STO,ASP,K, and aspheric coeffsA–I/J; constructsAperture,Spheric,Aspheric, setsd_sensor,r_sensor, andhfov.write_lens_seq()to emitSsurfaces (with material, curvature, thickness, aperture, aspheric data) andSIsensor.deeplens/geolens.py):.seqhandling inGeoLens.read_lens()viaread_lens_seq().Written by Cursor Bugbot for commit 332fa56. This will update automatically on new commits. Configure here.