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

Hacky NIRCAM/BAR_NARROW developments #114

Closed
wants to merge 6 commits into from
Closed

Hacky NIRCAM/BAR_NARROW developments #114

wants to merge 6 commits into from

Conversation

wbalmer
Copy link
Collaborator

@wbalmer wbalmer commented Dec 7, 2023

Take initial crpix from pysaif directly (ensures the narrow 400x256 subarray centers are recorded correctly in headers).
Options to take sqrt and mask whole regions before align and shift steps (both recenter_frames and align_frames). This enables the suppression of certain sidelobes and mask the central leakage (even if centers are recorded incorrectly), these effects can pull shifts away from small angle maneuver commands in the bar_narrow offset images.

I think all of this won't affect any normal usage :)

@wbalmer wbalmer requested a review from kammerje December 7, 2023 19:26
Copy link
Collaborator

@kammerje kammerje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @wbalmer already noted on Slack, populating the database with the pySIAF aperture reference position instead of the FITS file CRPIX header will not work if people crop or pad the images before in which case only the CRPIX header will be updated. Wrong CRPIX headers are a fault of the JWST pipeline and should be corrected there. This change needs to be undone. Tagging also @JarronL. SpaceKLIP is meant to be flexible and allow the user to determine the order in which steps are being run, and also stop/restart the reduction at any point in the process.

@JarronL
Copy link
Collaborator

JarronL commented Dec 8, 2023

As @wbalmer already noted on Slack, populating the database with the pySIAF aperture reference position instead of the FITS file CRPIX header will not work if people crop or pad the images before in which case only the CRPIX header will be updated. Wrong CRPIX headers are a fault of the JWST pipeline and should be corrected there. This change needs to be undone. Tagging also @JarronL. SpaceKLIP is meant to be flexible and allow the user to determine the order in which steps are being run, and also stop/restart the reduction at any point in the process.

The problem here is that CRPIX header values for the files that William is working on will never be corrected unless there is some external intervention. You are correct, this is a problem with the JWST fitswriter that generates the raw FITS files, but it may be a problem that persists for some time with a lot of bar masks data due to these new aperture sizes. In addition, with the introduction of simultaneous SW/LW, there are now incorrect APERNAME values in a number of coronagraphic datasets (due to incorrect fitswriter logic) that also have wrong CRPIX values. Suffice it to say, there's already a lot of data out in the wild with incorrect header information unbeknown to the user, so I would like SpaceKLIP to accommodate or at least warn about that.

I've already written a function that determines the correct aperture name based on the observations information (webbpsf_ext.imreg_tools.get_coron_apname), which allows us to then get the correct CRPIX values via pysiaf. Perhaps we should provide a standalone script that first checks the header for consistency and updates the FITS header values if they don't match the expected values?

@kammerje
Copy link
Collaborator

@JarronL I like the idea of having a separate script that checks and updates the header keywords if necessary. This way, the existing code will not break and people can still reduce their corrupt files. We could also introduce a new keyword when reading files into the database that allows the user to determine whether FITS header or pySIAF CRPIX values shall be used, with an explicit warning that the latter won't work if the data has been padded or cropped before.

@wbalmer
Copy link
Collaborator Author

wbalmer commented May 7, 2024

rebased this branch at Ananya's request

@asahooexo
Copy link

@wbalmer, I think the rebasing didn't go well as this branch is still 46 commits behind develop.

@wbalmer
Copy link
Collaborator Author

wbalmer commented Jun 9, 2024

Since this PR was based on my dev/wb branch and is a bit of a mess, I am closing it and opening two new PRs (#182 and #183 ) with their own dummy branches that address the two distinct changes I had to make to reduce NIRCam bar NARROW data.

@wbalmer wbalmer closed this Jun 9, 2024
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.

4 participants