Fix METIS IFU mode

[teutoburg]

Got a few errors when attempting to run the new laser_spectrum and pinhole_mask through the lms (aka IFU?) mode. This, together with the IRDB counterpart, makes it work and the output looks promising.

Short explanation on those new exceptions:
When I initially ran the simulation, I got weird errors which I ultimately traced down to a less-than-optimal handling of "Spectral trace outside FOV". Now the trace being outside the FOV was a symptom of something else (wrong detector layout), but that shouldn't have stopped the simulation as a whole (the erroneous layout basically mimicked a DetecorWindow, and that's supposed to work...). However the function map_spectra_to_focal_plane() just logged a warning/info and then return None (instead of a HDU), while the calling function had no way of handling this (unexpected but not impossible) None value, so it subsequently failed attempting to access the supposed HDU's attributes. Since I figured it would be fine to just "skip" a trace if it's outside the final FOV anyway, I added the new exceptions as a way for the calling function to deal with unexpected (but not catastrophic) behavior of the lower level. This also has the benefits of a) map_spectra_to_focal_plane() always returns a HDU or raises an exception, but never silently returns None; b) allows different treatment of different fail conditions if required (currently just handles the base exception).

All of this might also just be another symptom of the split between "normal" spectral traces and the "special" METIS classes.

[codecov]

Codecov Report

Attention: Patch coverage is 40.62500% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 74.92%. Comparing base (bc51dce) to head (cdd1a54).

Files	Patch %	Lines
scopesim/effects/metis_lms_trace_list.py	35.00%	13 Missing ⚠️
scopesim/effects/spectral_trace_list_utils.py	45.45%	6 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           dev_master     #391   +/-   ##
===========================================
  Coverage       74.92%   74.92%           
===========================================
  Files              56       56           
  Lines            7777     7781    +4     
===========================================
+ Hits             5827     5830    +3     
- Misses           1950     1951    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[teutoburg]

Requires AstarVienna/irdb#165 to fully work...

[teutoburg]

Coverage is complaining, because many of the changes here only happen in non-ordinary (mostly untested) conditions...

[hugobuddel]

Ah here you introduced the bug that you then fixed in AstarVienna/irdb#165 . We can merge that one if we also merge this one. Let me review the rest of this PR first though

[hugobuddel]

Removing this copy() seems rather dangerous, as now any instance of MetisLMSEfficiency can change the values of other instances. E.g.:

from scopesim.effects.metis_lms_trace_list import MetisLMSEfficiency
eff1 = MetisLMSEfficiency(wavelen=4.2, filename="METIS/TRACE_LMS.fits")
print(f"{eff1.meta['eff_max']=}")
eff2 = MetisLMSEfficiency(wavelen=4.2, filename="METIS/TRACE_LMS.fits", eff_max=42)
print(f"{eff2.meta['eff_max']=}")
print(f"{eff1.meta['eff_max']=}")
eff3 = MetisLMSEfficiency(wavelen=4.2, filename="METIS/TRACE_LMS.fits")
print(f"{eff3.meta['eff_max']=}")

will print

eff1.meta['eff_max']=0.75
eff2.meta['eff_max']=42
eff1.meta['eff_max']=42
eff3.meta['eff_max']=42

is that intentional?

[hugobuddel]

Maybe because you removed the copy() in __init__()? You directly set self.meta = self._class_params, which means that all MetisLMSEfficiency instances share the same .meta.

Just guessing. And since I'm already started with 'single comments', I'll keep doing that :-).

[hugobuddel]

My main request for changes is the removal of the copy(). If that change was intentional, then please add a comment why no copy is necessary, otherwise I will probably forget that and add the copy back in at some later date assuming it should have been there. If the removal of the copy() was not on purpose, than I suggest adding it back in.

Other than that, my philosophy is that "Exceptions are for exceptional circumstances"; so I personally lean more toward returning errors than raising exceptions, but maybe I should change my mind on that.

[teutoburg]

I'm starting to question what I was fixing here in the first place. Like, I'm certain there were some errors related to unresolved bang-strings for the layout filename or something like that. Which is how the whole file_name thing started. But I now fail to reproduce that error when attempting to run the same simulations from dev_master (both here and IRDB). Not sure what's happening here, but I'm more than a bit confused...

[teutoburg]

My main request for changes is the removal of the copy(). If that change was intentional, then please add a comment why no copy is necessary, otherwise I will probably forget that and add the copy back in at some later date assuming it should have been there. If the removal of the copy() was not on purpose, than I suggest adding it back in.

I think that change might have slipped in from another branch where I did some cmds/meta/params stuff. I'll revert that here and see if it solves the duplicate keyword thingy...