Closed schlafly closed 3 weeks ago
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 6 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
romanisim/image.py | 56.25% | 7 Missing :warning: |
romanisim/wcs.py | 82.35% | 3 Missing :warning: |
romanisim/ris_make_utils.py | 50.00% | 2 Missing :warning: |
romanisim/util.py | 97.14% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@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.,
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.
Hi Eddie,
thanks for working on this. I’m going to forward your email to Sanjib who worked on the metadata keywords.
Cheers,
Andrea
On Aug 16, 2024, at 11:34, Eddie Schlafly @.***> wrote:
@AndreaBellini https://urldefense.com/v3/__https://github.com/AndreaBellini__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoGfZQda1$, @tddesjardins https://urldefense.com/v3/__https://github.com/tddesjardins__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoKIw2UDP$ , this is intended to address #136 https://urldefense.com/v3/__https://github.com/spacetelescope/romanisim/issues/136__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoKurbwVi$ . Let me know if this addresses your concerns. As I mentioned in the comments on #136 https://urldefense.com/v3/__https://github.com/spacetelescope/romanisim/issues/136__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoKurbwVi$, 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. — Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/spacetelescope/romanisim/pull/142*issuecomment-2293727123__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoLrbqGhu$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AE37OQRAI2BGS4ASC2FRDITZRYLYTAVCNFSM6AAAAABMUEG4A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJTG4ZDOMJSGM__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!1u5_O3PwV3XJs0LPJRXfdZT0zwJR8z9HdVvvcR643ySTS_q-1cRiZbHz9iyqu8BDVyFzjMA_l5_TuWgVoDDcLfj1$. You are receiving this because you were mentioned.
-- Dr. Andrea Bellini Space Telescope Science Institute 3700 San Martin Drive Baltimore, MD, 21218, USA Office: +1 (410) 338-4431
I'm going ahead with this for now, but please let me know if you have any input.
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:
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).
I have left the photom one blank since it is not used.