Oc/line widths #213
Oc/line widths #213
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev_master #213 +/- ##
==============================================
+ Coverage 75.13% 75.23% +0.10%
==============================================
Files 147 147
Lines 15181 15255 +74
==============================================
+ Hits 11406 11477 +71
- Misses 3775 3778 +3
☔ View full report in Codecov by Sentry. |
|
These changes yield correct line widths and should close issue #212. The example is for an unresolved line from a slit-filling source, observed with METIS LSS L-band and three slits. The dashed lines mark the respective slit widths. |
There was a problem hiding this comment.
Thank you @oczoske for taking this on! I can mostly follow what is happening, so I only have two minor optional suggestions. Also thanks for cleaning up the file while you're at it.
| @@ -134,7 +141,7 @@ def map_spectra_to_focal_plane(self, fov): | |||
| The method returns a section of the fov image along with info on | |||
| where this image lies in the focal plane. | |||
| """ | |||
|
|
|||
| print("Mapping", fov.meta['trace_id']) | |||
There was a problem hiding this comment.
| print("Mapping", fov.meta['trace_id']) | |
| logging.info("Mapping %s", fov.meta['trace_id']) |
There was a problem hiding this comment.
Should be logging.info("Mapping %s", fov.meta["trace_id"]), logging functions take *args for the formatting.
There was a problem hiding this comment.
Yeah, nice, except that I don't see the message anymore (admittedly, it's purpose is to keep me awake by providing some progress info). I see that cmd['!SIM.logging.console_level'] is set to 'WARNING' (in scopesim's defaults.yaml, so I guess that would have to be changed explicitely. Do we have a policy on logging? I usually feel that scopesim is a bit too quiet when doing lengthy operations.
In any case, I adopt it with an associated reformulation of the warning on l.177.
| @@ -457,12 +457,17 @@ def __init__(self, fov, dlam_per_pix): | |||
| # Initialise the array to hold the xi-lambda image | |||
| self.image = np.zeros((n_xi, n_lam), dtype=np.float32) | |||
| self.lam = cube_lam | |||
| try: | |||
| dlam_per_pix_val = dlam_per_pix(np.asarray(self.lam)) | |||
| except TypeError: | |||
There was a problem hiding this comment.
When would this exception occur? XiLamImage is only initialized once in ScopeSim, earlier in this file. There dlam_per_pix is always set to the output of interp1d, which is always a function I think. So it seems this except block can be removed, which would clarify better what's going (perhaps with type hints on the parameters).
There was a problem hiding this comment.
This was meant to preserve the previous behaviour where dlam_per_pix was passed as just one number (the average dispersion). Although in the current implementation this should not occur, I wouldn't exclude that XiLamImage could be used in a different context in the future.
XiLamImagenow accepts a dispersion functiondlam_per_pixinstead of averageavg_dlam_per_pix(a constant is still accepted)xy2lamin the dispersion direction.