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

Metadata updates for science verification. #142

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

schlafly
Copy link
Collaborator

@schlafly schlafly commented Aug 16, 2024

This PR makes a number of metadata changes for science verification. The pixels are not affected. It is intended to address #136 .

Some of the changes:

  • There is now a "version" keyword in the 'romanisim' section of the output:
>>> asdf.open('l1.asdf')['romanisim']['version']
'0.5.3.dev9+g273099b'
  • guide window metadata is set to size 16
>>> asdf.open('l1.asdf')['roman']['meta']['guidestar']['gw_window_xsize']
16
>>> asdf.open('l1.asdf')['roman']['meta']['guidestar']['gw_window_ysize']
16
  • photometry metadata is now -999999 for L1 files but has sensible values for L2 files.
>>> asdf.open('l2.asdf')['roman']['meta']['photometry']
{'conversion_microjanskys': <Quantity 15.11015947 uJy / arcsec2>, 'conversion_megajanskys': <Quantity 0.64286431 MJy / sr>, 'pixelarea_steradians': <Quantity 2.80827472e-13 sr>, 'pixelarea_arcsecsq': <Quantity 0.01194785 arcsec2>, 'conversion_megajanskys_uncertainty': <Quantity 0. MJy / sr>, 'conversion_microjanskys_uncertainty': <Quantity 0. uJy / arcsec2>}

Note these values will not match the romancal versions in detail; they use the bandpass we get from Goddard rather than the CRDS numbers. Those have a 3% difference (plus a ~factor of two gain difference, since the CRDS versions are for e/s instead of DN/s).

  • The ref files get populated in the L1 and L2 files:
>>> asdf.open('l2.asdf')['roman']['meta']['ref_file']
{'crds': {'context_used': 'roman_0063.pmap', 'sw_version': '11.17.19'}, 'dark': 'roman_wfi_dark_0896.asdf', 'distortion': 'roman_wfi_distortion_0064.asdf', 'flat': 'roman_wfi_flat_0200.asdf', 'gain': 'roman_wfi_gain_0186.asdf', 'linearity': 'roman_wfi_linearity_0314.asdf', 'mask': 'N/A', 'photom': 'N/A', 'readnoise': 'roman_wfi_readnoise_0748.asdf', 'saturation': 'roman_wfi_saturation_0218.asdf'}

I have left the photom one blank since it is not used.

  • The WCS for romanisim cannot really be wrong relative to romancal, since defines the wcs used to do the simulation. The romancal ELP WCS gets run through tweakreg and updated based on an attempt to match detected stars with real stars; romanisim doesn't attempt to simulate this and just outputs the WCS it uses. I don't think any changes here make sense. I did however update the roll_ref used for the wcsinfo as part of this PR to use the angles relative to north at the reference points rather than at WFI_CEN.
  • I added s_region. I am not able to find a definition for this quantity. I am using the coordinates of the centers of the corner pixels. I picked a different pixel order than romancal, and am using the centers of the corner pixels, while romancal (and likely Webb?) are using the centers of the zeroth row and column of pixels, but the centers of a virtual one-past-the-edge pixel. This gives the s_region the same physical size as the device would have on the sky, but shifted by half a pixel relative to its boundaries, while the convention I'm using is a pixel smaller but is correctly centered. If you have opinions about the exact convention for s_region, please advise.
>>> asdf.open('l2.asdf')['roman']['meta']['wcsinfo']['s_region']
'POLYGON ICRS 269.98690833079274 65.97427994844489 269.6801513833561 65.97435689705982 269.67682107205064 66.09718041146162 269.9867021551381 66.09721332657264'
  • I have updated the shapes of the empty, unused border pixel arrays in the L2 product to match expectations from read_pattern.
  • I have not addressed changing the L2 metadata to reflect future. That is beyond the scope of romanisim science validation.

@schlafly schlafly requested a review from a team as a code owner August 16, 2024 14:07
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 82.43243% with 13 lines in your changes missing coverage. Please review.

Project coverage is 89.53%. Comparing base (d4af8fd) to head (32896b0).
Report is 51 commits behind head on main.

Files with missing lines Patch % Lines
romanisim/image.py 56.25% 7 Missing ⚠️
romanisim/wcs.py 82.35% 3 Missing ⚠️
romanisim/ris_make_utils.py 50.00% 2 Missing ⚠️
romanisim/util.py 97.14% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #142      +/-   ##
==========================================
- Coverage   89.95%   89.53%   -0.42%     
==========================================
  Files          17       17              
  Lines        1792     1883      +91     
==========================================
+ Hits         1612     1686      +74     
- Misses        180      197      +17     

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


🚨 Try these New Features:

@schlafly
Copy link
Collaborator Author

@AndreaBellini, @tddesjardins , this is intended to address #136 . Let me know if this addresses your concerns. As I mentioned in the comments on #136, there are a lot of differences in detail. i.e.,

  • I don't use the CRDS values for the photometry metadata because they are wrong and not what romanisim uses (we need full bandpass information and must compute this ourselves anyway). We do now populate these keywords. I don't know how the pixel area values were calculated and calculate this myself; in detail the value in CRDS is not the center, mean, or median pixel's area; I use the center pixel's area.
  • I now populate the ref_file information when used, except for the photom file, which we don't use.
  • romancal runs tweakreg to match the WCS to the image. That of course is noisy and so we can't expect the WCSes to match exactly. romanisim defines the correct WCS and so it is weird to say that the romanisim WCS should match the romancal WCS. I did change some of the wcsinfo metadata so that romancal exactly produces the romanisim wcs if tweakreg is skipped.
  • I now populate s_region. It's not clear to me how exactly this quantity is defined (edges of pixels? centers of pixels?). We don't match romancal at the level of those details but are within a pixel and ordering conventions.
  • The unused, unsimulated border pixels in the L2 files now have the shape one would expect given the read pattern.

@tddesjardins
Copy link
Collaborator

Thank you for the update! I'll leave it to Andrea to focus on the rest. As for the zeropoints and transmission info, I'm trying to get the even more latest ones from GSFC right now (not the March ones, but post-TVAC2) and add transmission info into the reference files while simultaneously doing an update to the zeropoints. I think we can address some of that issue longer-term since it will likely be little bit before we get that info, but I'm trying.

@AndreaBellini
Copy link

AndreaBellini commented Aug 16, 2024 via email

@schlafly
Copy link
Collaborator Author

I'm going ahead with this for now, but please let me know if you have any input.

@schlafly schlafly merged commit 7bf3577 into spacetelescope:main Aug 26, 2024
20 of 22 checks passed
@schlafly schlafly deleted the more-metadata-changes branch August 26, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants