Added implementation of filter refraction as a photon operator#103
Added implementation of filter refraction as a photon operator#103FedericoBerlfein wants to merge 3 commits into
Conversation
sidneymau
left a comment
sidneymau
left a comment
There was a problem hiding this comment.
Left a few comments. The math throughout looks reasonable, but I'd have to dedicate a bit more time to fully review everything here
|
|
||
|
|
||
| # =========================================================================== | ||
| # Roman WFI instrument constants |
There was a problem hiding this comment.
is there a global reference these can come from? Not sure if this info exists already in romanisim or somewhere else? (cc @yuedong0607)
| def n_Suprasil3001(lam_nm): | ||
| """ | ||
| Refractive index of the Roman filter substrate (Suprasil 3001) | ||
| Coefficients B and C come from Sellmeier dispersion formula. |
There was a problem hiding this comment.
I would suggest including a link to the dispersion formula (e.g., https://en.wikipedia.org/wiki/Sellmeier_equation). The coefficients come from the Heraeus document, not from the formula
There was a problem hiding this comment.
Maybe it would be cleaner to have a separate sellmeier_dispersion function that accepts the lam_nm and the various constants as input. then this function just calls that, passing in the appropriate constants
| return x_fpa, y_fpa | ||
|
|
||
|
|
||
| def getAOI(sca, x_pos, y_pos): |
There was a problem hiding this comment.
I think these should be x_sci/y_sci for consistency with _sca_to_fpa_coords?
| self.sca = SCA | ||
| self.sca_pos = SCA_pos if SCA_pos is not None else galsim.PositionD(2044, 2044) | ||
| self.eff_wave_nm = bandpass.effective_wavelength | ||
| self.n = n if n is not None else n_Suprasil3001 |
There was a problem hiding this comment.
is there a reason that some of these aren't just the default argument in the function signature? is this to elide galsim argument parsing/validation?
| return RomanFilterRefraction( | ||
| bandpass=bandpass, | ||
| SCA=sca, | ||
| SCA_pos=galsim.PositionD(pos.x, pos.y), |
There was a problem hiding this comment.
from the comment, on L566, I thought pos was already a PositionD. is there a reason to unwrap/rewrap like this?
2 participants
Implementation of refraction from the WFI filters as a photon operator. The photon operator lives in the file
filter_refraction.pyand contains the relevant functions used in the photon operator. Modifications to__init__.pyare simply to add the new file and functions within it. Modifications toconfig/default.yamlare to add refraction prior to charge diffusion. I believe this is the correct order, as this effect occurs before photons reach the detector.A small breakdown of the functions in
filter_refraction.py:class RomanFilterRefraction: This class contains the GalSim photon operator. To initialize the operator, you need to specify the bandpass, SCA and SCA position, pixel scale, and callable function to get the index of refraction as a function of wavelength. TheapplyTogets the lateral chromatic shifts induced by filter refraction and applies it directly to the photons based on their wavelength. The default for the pixel scale is the native roman scale (0.11 arcsec/pixel), and the default index of refraction comes from the Roman WFI filter substrate (Suprasil 3001)class RomanFilterRefractionBuilder: This class builds the photon operator from the information inbase, which includes the bandpass, SCA and SCA position._get_lateral_shifts: This is the main function called withinapplyTothat calculates the lateral chromatic shifts. This function calls other helper functions defined throughout the file.pysiaf)