Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes default values for PSF (Point Spread Function) kernel size and wavelength parameters across the codebase by introducing and using shared constants (PSF_KS = 64 and DEFAULT_WAVE = 0.587). The changes also include comprehensive documentation improvements with expanded docstrings for key methods, code formatting enhancements, copyright header additions, and minor refactoring for better maintainability.
Key Changes:
- Standardized PSF kernel size to use
PSF_KS(64) constant across the codebase instead of hardcoded values (31, 51, 101, etc.) - Updated default wavelength parameters to use
DEFAULT_WAVEconstant for consistency - Enhanced documentation with detailed docstrings for initialization, I/O, analysis, and optimization methods in
DiffractiveLens,HybridLens, andGeoLensclasses - Added copyright headers to multiple geometry surface files and improved code formatting
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_monte_carlo.py | Updated test PSF kernel size from 51 to 64 to align with PSF_KS constant |
| test/test_geolens.py | Updated test PSF kernel sizes from 31/51 to 64, added new dispatcher tests for coherent and Huygens PSF models |
| docs/user_guide/lens_systems.rst | Updated documentation examples to use PSF_KS (64) instead of hardcoded 51 |
| docs/tutorials.rst | Updated tutorial examples to use standardized PSF_KS value (64) |
| docs/quickstart.rst | Enhanced quickstart guide with examples of different PSF models (geometric, coherent, huygens) |
| docs/examples/image_simulation.rst | Fixed parameter name from point to points for coherent PSF |
| docs/api/lens.rst | Updated API documentation to reflect PSF_KS and DEFAULT_WAVE constants |
| docs/api/hybridlens.rst | Updated HybridLens API docs with standardized constants and SPP_COHERENT |
| docs/api/geolens.rst | Expanded GeoLens PSF documentation with detailed parameter descriptions and new PSF model types |
| deeplens/psfnetlens.py | Updated psf_map_rgb to use PSF_KS constant and corrected docstring |
| deeplens/paraxiallens.py | Standardized all PSF-related methods to use PSF_KS constant |
| deeplens/optics/wave.py | Relaxed wavelength assertion from < 1.0 to < 10.0 μm for broader wavelength support |
| deeplens/optics/ray.py | Relaxed wavelength assertion, reformatted code for better readability |
| deeplens/optics/psf.py | Updated solve_psf functions to use PSF_KS constant |
| deeplens/optics/materials.py | Changed Cauchy equation wavelength reference from 0.589 to 0.587 μm to align with DEFAULT_WAVE |
| deeplens/optics/geometric_surface/thinlens.py | Minor docstring update ("in air" → "are air") |
| deeplens/optics/geometric_surface/spiral.py | Added copyright header |
| deeplens/optics/geometric_surface/qtype.py | Added copyright header |
| deeplens/optics/geometric_surface/prism.py | Added copyright header |
| deeplens/optics/geometric_surface/aspheric_norm.py | Added construct_ai helper method for converting normalized to absolute coefficients |
| deeplens/lens.py | Updated base Lens class PSF methods to use DEFAULT_WAVE and PSF_KS constants |
| deeplens/hybridlens.py | Reorganized imports, added comprehensive docstrings for all major methods, improved code formatting |
| deeplens/geolens.py | Extensively improved documentation with detailed docstrings for initialization, ray tracing, rendering, PSF calculation, and optical analysis methods |
| deeplens/diffraclens.py | Enhanced docstrings for example loaders, I/O methods, and visualization functions; standardized to use PSF_KS and DEFAULT_WAVE |
| configs/4_tasklens.yml | Updated psf_ks from 101 to 64 to match PSF_KS standard |
| 6_hybridlens_design.py | Updated PSF kernel size from 101 to 128 (deviates from PSF_KS standard) |
Comments suppressed due to low confidence (2)
deeplens/lens.py:619
- Overridden method signature does not match call, where it is passed too many arguments. Overriding method method GeoLens.get_optimizer_params matches the call.
Overridden method signature does not match call, where it is passed an argument named 'lrs'. Overriding method method GeoLens.get_optimizer_params matches the call.
Overridden method signature does not match call, where it is passed an argument named 'decay'. Overriding method method GeoLens.get_optimizer_params matches the call.
Overridden method signature does not match call, where it is passed an argument named 'optim_mat'. Overriding method method GeoLens.get_optimizer_params matches the call.
Overridden method signature does not match call, where it is passed an argument named 'optim_surf_range'. Overriding method method GeoLens.get_optimizer_params matches the call.
def get_optimizer_params(self, lr=[1e-4, 1e-4, 1e-1, 1e-3]):
deeplens/psfnetlens.py:319
- This method does not accept arbitrary keyword arguments, which overridden Lens.psf_rgb does. This call correctly calls the base method, but does not match the signature of the overriding method.
def psf_rgb(self, points, ks=64):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Cauchy's equation, calculate (A, B) on the fly | ||
| B = (self.n - 1) / self.V / (1 / 0.486**2 - 1 / 0.656**2) | ||
| A = self.n - B * 1 / 0.589**2 | ||
| A = self.n - B * 1 / 0.587**2 |
There was a problem hiding this comment.
The wavelength value changed from 0.589 to 0.587 in the Cauchy equation for optimizable materials. However, the standard d-line wavelength for sodium is 0.5893 μm, so 0.589 is more accurate. While this change aligns with the DEFAULT_WAVE constant (0.587 μm), it introduces a discrepancy with the optical standard. Consider either keeping 0.589 here for accuracy or updating DEFAULT_WAVE to 0.589 to match the standard d-line wavelength.
| A = self.n - B * 1 / 0.587**2 | |
| A = self.n - B * 1 / 0.589**2 |
| pbar = tqdm(total=iterations + 1, desc="Progress", postfix={"loss": 0}) | ||
| for i in range(iterations + 1): | ||
| psf = lens.psf(points=[0.0, 0.0, -10000.0], ks=101, wvln=0.489) | ||
| psf = lens.psf(points=[0.0, 0.0, -10000.0], ks=128, wvln=0.489) |
There was a problem hiding this comment.
The PSF kernel size is changed to 128, which differs from the PSF_KS constant (64) that is being standardized throughout the codebase. For consistency with the standardization effort described in the PR, this should use PSF_KS constant or a multiple of it with clear justification. Consider using ks=PSF_KS or documenting why 128 is needed here specifically.
| # 3. PSF radial | ||
| # =========================================== | ||
| def psf(self, points, wvln=0.589, ks=51, **kwargs): | ||
| def psf(self, points, wvln=DEFAULT_WAVE, ks=PSF_KS, **kwargs): |
There was a problem hiding this comment.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method GeoLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'spp'. Overriding method method GeoLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'recenter'. Overriding method method GeoLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
Overridden method signature does not match call, where it is passed an argument named 'psf_type'. Overriding method method ParaxialLens.psf matches the call.
| return img_render | ||
|
|
||
| def psf(self, depth=float("inf"), wvln=0.589, ks=101, upsample_factor=1): | ||
| def psf(self, depth=float("inf"), wvln=DEFAULT_WAVE, ks=PSF_KS, upsample_factor=1): |
There was a problem hiding this comment.
This method does not accept arbitrary keyword arguments, which overridden Lens.psf does. This call correctly calls the base method, but does not match the signature of the overriding method.
This method does not accept keyword argument points, which overridden Lens.psf does. This call correctly calls the base method, but does not match the signature of the overriding method.
| return img_render | ||
|
|
||
| def psf(self, depth=float("inf"), wvln=0.589, ks=101, upsample_factor=1): | ||
| def psf(self, depth=float("inf"), wvln=DEFAULT_WAVE, ks=PSF_KS, upsample_factor=1): |
There was a problem hiding this comment.
Overriding method signature does not match here, where it is passed an argument named 'points'. Overridden method method Lens.psf is correctly specified.
This pull request standardizes the default values for PSF (Point Spread Function) kernel size and wavelength parameters across the codebase by using shared constants (
PSF_KSandDEFAULT_WAVE). It also improves code documentation by expanding and clarifying docstrings for key methods in bothdeeplens/diffraclens.pyanddeeplens/hybridlens.py. Additionally, minor code cleanups and reordering of imports are performed for better readability and maintainability.Parameter Standardization:
psfand related methods to usePSF_KSandDEFAULT_WAVEconstants indeeplens/diffraclens.py,deeplens/hybridlens.py, anddeeplens/lens.pyfor consistency. [1] [2] [3] [4] [5]configs/4_tasklens.ymlto use the new standardized PSF kernel size value.6_hybridlens_design.pyto match new standard.Documentation Improvements:
load_example1,load_example2,read_lens_json,write_lens_json,draw_layout, anddraw_psfindeeplens/diffraclens.py. [1] [2] [3] [4] [5] [6]deeplens/hybridlens.py. [1] [2] [3] [4] [5] [6]Code Cleanups and Refactoring:
deeplens/hybridlens.pyfor clarity.These changes make the codebase more maintainable, easier to understand, and ensure consistent behavior when working with PSF-related computations.