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

DM-6147 #65

Merged
merged 2 commits into from May 23, 2016
Merged

DM-6147 #65

merged 2 commits into from May 23, 2016

Conversation

r-owen
Copy link
Contributor

@r-owen r-owen commented May 19, 2016

No description provided.

It turns out that IsrTask masks saturated pixels after the image
is converted to floating point. Furthermore, the task does no masking
if the saturation level is `nan`. However, the saturation level
was an integer, so I changed the saturation level to a float
to match the task code.

Also document `nan` to mean no level in the help for the field.
@r-owen r-owen changed the title Tickets/dm 6147 DM-6147 May 19, 2016
@parejkoj
Copy link
Contributor

These look good. The only thing I might add would be a clarification on what "suspect" means in the help description. Is that a term of art in LSST land, or is it supposed to be a general catch-all for "some undefined thing may be wrong here"?

@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

That is an excellent point. I will expand the explanation. For the record: it means a pixel whose value is high enough that the non-linearity correction may not work very well, and so very picky algorithms may wish to avoid using this pixel. I worry it's too granular, but HSC apparently finds it useful.

@parejkoj
Copy link
Contributor

Ah, then can we call it something other than "suspect"? Because that sounds like a more generic thing. Maybe "non-linear suspect" or something?

@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

Ack!!!!!

@parejkoj
Copy link
Contributor

I'm sorry about the naming complaint, but I was definitely confused by it when I saw the name, and I could see wanting to introduce a "suspect" mask in the future as a catch-all.

@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

I agree it's not a great name, but the mask plane is called SUSPECT so I think it's the best choice unless we rename the mask plane (and I don't want to do that unless it's really important)

@parejkoj
Copy link
Contributor

Ok. Just document the name well.

@parejkoj
Copy link
Contributor

The new description is definitely better. Thanks.

@RobertLuptonTheGood
Copy link
Member

I like SUSPECT, as we may set it for other sorts of problems too.

@parejkoj
Copy link
Contributor

If we're defining "suspect" as "pixels with values >= suspectLevel may be poorly linearized", then we shouldn't use "suspect" for other problems. If we want to use it for other problems, we should define it as "pixels whose post-ISR values are suspect for various reasons." And if we do that, the definition of suspectLevel will be meaningless, since it's not a value >= suspectLevel test anymore.

@RobertLuptonTheGood
Copy link
Member

I think we're using suspect as "pixels that we don't trust". One type of untrusted pixel is those above a certain level, so suspectLevel is OK. I think I'm agreeing with John, but I'm not quite sure

@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

Robert: could you please provide a definition of suspect/suspect level?

@parejkoj
Copy link
Contributor

That was what I initially suspected (see what I did there?) suspect meant, but in this context it appears that it has specific meaning. The documentation should be clear about this: possibly isrTask says what it currently says (because that's how suspect is used in that context) while afw says the more generic thing (suspect means we don't trust it).

@RobertLuptonTheGood
Copy link
Member

Does this help?

Pixels whose values may be affected by unknown systematics are labelled SUSPECT. An example is that the NAOJ instrument scientist for SuprimeCam recommends not trusting the values of pixels above a certain level (this could happen if the non-linearity corrections above this level are unstable). We accordingly set the SUSPECT mask bit for all pixels above a level, suspectLevel -- which may be infinite for the LSST electronics. In the future we may identify other sources of suspect pixels.

@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

Thank you Robert. That is very helpful.

Add new scalar to AmpInfoCatalog; the level above
which pixels are masked as SUSPECT (or nan to not mask).
@r-owen
Copy link
Contributor Author

r-owen commented May 20, 2016

Here is the description I came up with for the amp info catalog field (pushed to the ticket):

level above which pixels are considered suspicious, meaning they may be affected by unknown
systematics; for example if non-linearity corrections above a certain level are unstable
then that would be a useful value for suspectLevel; use `nan` if no such level applies

@parejkoj
Copy link
Contributor

I'm not sure about the "level above which" part. Robert's description above suggests that there could be other reasons for a pixel to be suspicious (I think? @RobertLuptonTheGood ).

@r-owen
Copy link
Contributor Author

r-owen commented May 23, 2016

In support of "level above": if pixels with low signal are hard to correct accurately then we are in deep trouble, since much of our signal is faint. I suggest we keep "level above" for now. It's probably sufficient, and if not, I suspect we will need to do much more work.

@r-owen r-owen merged commit df04471 into master May 23, 2016
@ktlim ktlim deleted the tickets/DM-6147 branch August 25, 2018 06:44
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.

None yet

3 participants