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

[Bug]: If a user marks a column as saturated in every group in S1, the column is still used in S3 #597

Open
2 tasks done
erinmmay opened this issue Dec 20, 2023 · 5 comments
Assignees
Labels
bug Something isn't working NIRSpec

Comments

@erinmmay
Copy link
Collaborator

FAQ check

  • Yes, I checked the FAQ and my question has not been addressed.

Instrument

NIRSpec (Stages 1-3)

What happened?

From a chat with @gianninapr - Currently Stage 1 has the option for a user to define which columns are saturated in which group so they can be ignored for GLBS and ramp fitting. However in some cases a user may want to manually flag a column as saturated in every group. In this case, the column isn't ignored in Stage 3 because there we only ignore pixels with an odd DQ (or bad pixels) and saturated pixels are not necessarily flagged as bad pixels. This means the Stage 3 code tries to use them for the median frame etc. We should add a check in the Stage 1 saturation flagging code so that columns flagged as saturated in every group get an updated DQ value so that they are also flagged as bad pixels.

Along the same lines: allow a user to supply a custom DQ mask in Stage 1. I'll include that option in a PR to address this bug.

Error traceback output

No response

What operating system are you using?

No response

What version of Python are you running?

No response

What Python packages do you have installed?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@erinmmay erinmmay added the bug Something isn't working label Dec 20, 2023
@erinmmay erinmmay changed the title [Bug]: If a user marks a column as saturated in every group in S1, the pixel is still used in S3 [Bug]: If a user marks a column as saturated in every group in S1, the column is still used in S3 Dec 20, 2023
@gianninapr
Copy link
Collaborator

Some notes on this issue:

After my conversation with Erin, I tried running a quick fix, but the results were still not ideal. After going over the definitions of the masks and a conversation with Caleb Cañas we both noticed the issue as it stands.

In update_saturation.py around line 46 the user-provided custom saturation columns are set to the sat_flag value, which is defined as the 'SATURATED' jwst dq flag. When the flags are passed in s3_reduce.py, the code marks all odd entries as do not use, yet the dq flag for 'SATURATED' is an even number (2), so it isn't marking the user-defined saturated columns as do not use, which is why we still see them on the median frame in the end.

I'm happy to take care of this if there's a straightforward fix (like adding a condition to also catch the saturation flag in the if statement in s3_reduce.py) that would be sufficient, but just wanted to check in and see if there was more thoughts people had and what they thought an elegant solution is.

@erinmmay
Copy link
Collaborator Author

My plan is to add a check in stage 1's update_saturation.py to flag a pixel as bad (odd number) if the user marks it as saturated in all groups. Some pixels may be marked as saturated in only later groups and still have a good ramp fit, so we don't want all saturated pixels ignored in stage 3.

@gianninapr gianninapr self-assigned this Dec 22, 2023
@gianninapr
Copy link
Collaborator

Given that the update_saturation.py script removes any saturation flags if they're present in the first group for use in ramp fitting, how would we want to update given that portion of the code? Do we just make sure the fully saturated pixels are marked as do not use before that step?

@taylorbell57 taylorbell57 added this to To do in Stage 1: Detector Processing via automation Feb 6, 2024
@taylorbell57 taylorbell57 added this to To do in Road to v1.0 via automation Feb 6, 2024
@taylorbell57 taylorbell57 moved this from To do to In progress in Road to v1.0 Feb 26, 2024
@taylorbell57
Copy link
Collaborator

@gianninapr and @erinmmay, do one of you have a PR to resolve this issue coming soon?

@gianninapr
Copy link
Collaborator

Yes, I was hoping to have a PR for this before our next meeting. I've had some conversations with Caleb on how to go about it!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working NIRSpec
Development

No branches or pull requests

4 participants