Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

METIS WCU effects #494

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open

METIS WCU effects #494

wants to merge 44 commits into from

Conversation

oczoske
Copy link
Collaborator

@oczoske oczoske commented Nov 5, 2024

This pull request includes Scopesim functionality to describe the METIS warm calibration unit. The idea is to have an empty source (empty_sky, taken as the default of OpticalTrain.observe, cf. #483), and add the emission at the exit of the integrating sphere as (subclasses of) TERCurve (BlackBodySource and LaserSource). Further effects describe the aperture masks (including thermal emission from the opaque parts).

The computation of the integrating-sphere exit radiance follows Roy van Boekel's radiometric model described in E-REP-MPIA-1203.

Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 72.05479% with 102 lines in your changes missing coverage. Please review.

Project coverage is 76.82%. Comparing base (a43ac16) to head (3097763).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
scopesim/effects/metis_wcu/metis_wcu.py 72.03% 59 Missing ⚠️
scopesim/effects/ter_curves.py 43.90% 23 Missing ⚠️
scopesim/effects/metis_wcu/fpmask.py 83.82% 11 Missing ⚠️
scopesim/utils.py 66.66% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #494      +/-   ##
==========================================
- Coverage   77.00%   76.82%   -0.19%     
==========================================
  Files          66       69       +3     
  Lines        8216     8573     +357     
==========================================
+ Hits         6327     6586     +259     
- Misses       1889     1987      +98     

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

@teutoburg teutoburg added enhancement New feature or request instrument-specific effects Related to a ScopeSim effect labels Nov 5, 2024
@oczoske oczoske marked this pull request as ready for review January 14, 2025 20:36
@oczoske
Copy link
Collaborator Author

oczoske commented Jan 14, 2025

I open the branch for review, although I don't intend to merge just yet. There is a problem with the fits keywords -- it's looking for telescope keywords where there are none. I may have to merge the corresponding irdb branch AstarVienna/irdb#205 first.

@oczoske oczoske requested review from teutoburg and astronomyk and removed request for teutoburg January 14, 2025 20:44
@hugobuddel
Copy link
Collaborator

I open the branch for review, although I don't intend to merge just yet.

I was about to ask 🤓

There is a problem with the fits keywords -- it's looking for telescope keywords where there are none.

Oh yeah, these broken header keywords is to be expected. And I did.

The idea was/is to split up https://github.com/AstarVienna/irdb/blob/dev_master/METIS/headers/FITS_common_keywords.yaml into more yaml files, and include more ExtraFitsKeyword effects in other places. I did not split this up, because I was like, we can do that when we need it.

In this case, all the telescope-related header keywords should be in e.g. FITS_telescope_keywords.yaml, which is then used in a ExtraFitsKeyword effect that should be placed in a separate yaml file (ELT_keywords.yaml?) that is imported only when ELT.yaml is imported in AstarVienna/irdb#205 .

Unfortunately it is not possible to place these keywords in the ELT package, because other ELT instruments will want different keywords. So that means that there should be an extra yaml file in METIS that just includes a single ExtraFitsKeywords.

Other solutions are possible, like just ignore anything that cannot be found, but that can easily lead to mistakes, so I deliberately did not add such a feature.

@hugobuddel
Copy link
Collaborator

Unfortunately it is not possible to place these keywords in the ELT package

Or maybe we can put this ExtraFitsKeywords effect in the ELT package, and refer to the actual filename with the keywords in a similar way as we do for the ter-curve of the mirrors?

That is, have some default keywords provided by the ELT package itself (or an empty set of keywords as default) and then a mechanism to override that default with a keyword yaml file in METIS and MICADO?

That would be cleaner than a separate yaml file in METIS just to include the METIS-specific ExtraFitsKeywords effect.

@hugobuddel
Copy link
Collaborator

Oh I should read first. Or maybe just shut up at 22:00, because that FITS header discussion does not directly apply to these test failures. Nevertheless, a similar solution can be applied here too.

However, I would expect a similar problem to happen in AstarVienna/irdb#205 for the fits keywords in the TEL hierarchy. Strange that it doesn't. Maybe I'll look at it later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effects Related to a ScopeSim effect enhancement New feature or request instrument-specific
Projects
Status: 👀 Awaiting Review
Development

Successfully merging this pull request may close these issues.

3 participants