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

Use proseco for a quasi-independent hot pix check #276

Merged
merged 10 commits into from
Jan 17, 2019
Merged

Use proseco for a quasi-independent hot pix check #276

merged 10 commits into from
Jan 17, 2019

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jan 12, 2019

Use proseco for a quasi-independent hot pix check

This feeds the stars and mags from the starcheck-determined star catalog into a small routine that calls proseco.guide.get_imposter_mags and determines the worst-case centroid offset for the
candidate star and the brightest 2x2 imposter. Those offsets are then checked against tolerances and warnings are output (> 2.5 arcsec centroid offset gives orange warn, > 4.0 offset gives red/critical warn).

Closes #244

Star.Body.Pixels.BadPixels = [ ...
-512, 511, -1, 0; ...
-1, 0, -512, 511; ...
-245, 0, 454, 454; ...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent here is that only the large CCD trap remains (not sure if the fid defect should be added).

Functionally I just checked that an observation that previously had a star with a trap-related warning still did so with this bad pixel list. Obsid 48812 in OCT2918A

>> WARNING: [ 8] Nearby ACA bad pixel.  row, col (-64, 454), dy, dz (0, 6) 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not add a comment in the file about the trap as I didn't want to play games with the parser.

@jeanconn jeanconn requested a review from taldcroft January 12, 2019 18:01
@taldcroft
Copy link
Member

What worries me here is that this check gives an OK to a case with a 7.0 mag star and a 8.01 mag imposter that could be at +3, +3 row/col relative to the nominal position. So basically that would be a total fail of a guide star. Either very large centroid offsets or pounding the background pixels.

So one can respond by making dmag=3.0, but then that ends up failing basically every 10.2 mag star.

I think what we run into here is the same issue that we solved for spoilers, namely one needs to look at centroid offsets that result. Of course for an imposter this is a little trickier because (from the star readout perspective) the imposter is dithering around.

So that's where in the meeting I had the reaction "let's check whatever proseco is normally checking for imposters". And having now just checked the code, it seems like a reasonable solution would just be re-casting this in terms of get_pixmag_for_offset(cand_mag, offset) and having the relevant check parameter be the allowed centroid offset.

This does not address the imposter getting into the background. For now we can live with that (esp. since the proseco selection process also does not account for this issue).

starcheck/src/lib/Ska/Starcheck/Obsid.pm Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved
@jeanconn
Copy link
Contributor Author

jeanconn commented Jan 15, 2019

I'm a little confused though. It sounds like you think get_pixmag_for_offset is smarter than it is, because I think basically, because it doesn't move the bad pixel around and find any kind of worst case, and just estimates that putting all the counts from the pixel on the edge of the 6x6 is the worst case, for a centroid offset of 4.5 arcsecs you get basically a fixed delta mag relationship of around 1 mag.

Or maybe the math for get_pixmag_for_offset is just wrong, but:

In [4]: proseco.guide.get_pixmag_for_offset(7, 4.5)
Out[4]: 7.9199419632364858
In [5]: proseco.guide.get_pixmag_for_offset(8, 4.5)
Out[5]: 8.9199419632364858
In [6]: proseco.guide.get_pixmag_for_offset(9, 4.5)
Out[6]: 9.9199419632364858
In [7]: proseco.guide.get_pixmag_for_offset(10, 4.5)
Out[7]: 10.919941963236486
In [8]: proseco.guide.get_pixmag_for_offset(11, 4.5)
Out[8]: 11.919941963236486

@jeanconn
Copy link
Contributor Author

jeanconn commented Jan 15, 2019

The guide star selection basically has the "hope" that a less spoiled star than the 7.0 mag star with the 8.01 mag imposter in your scenario is selected, but does not really have explicit protection against it. (thought there is some protection for the 7.0 mag star in that the current dark current and temperature range show no 8.01 mag imposters).

@taldcroft
Copy link
Member

taldcroft commented Jan 16, 2019

So I didn't appreciate that the last-stage centroid offset is so large, 4.5 arcsec. I would consider that entirely unacceptable for a guide star. Your code examples bear this out - I had said a delta mag of 1.0 is no good, and now you say that a delta mag of 1.0 corresponds to a centroid offset that is likewise no good. So we're on the same page.

But it has led me to an idea for hot-pixel checking that might work. Well at least it makes sense to me at this moment: for the brightest identified imposter, compute the max centroid offset assuming the imposter is at the edge (same as get_pixmag_for_offset). Take the max of the y and z offsets. Then:

  • Critical warning for offset > 4.0 arcsec (basically useless guide star)
  • Else Orange warning for offset > 2.5 arcsec (would rather not use it)

So with this, the checking is keyed off a physically meaningful test (even though we accept that the computation of the offset has limitations). The test leverages proseco code but has independent limits.

@jeanconn
Copy link
Contributor Author

Gotcha. WRT "I would consider that entirely unacceptable for a guide star. " IIRC, we bumped it out that far with the idea that up to the 5 arcsec limit is presently not unusual for OBC centroid residuals (well, and we were planning on more annie help). We could also warn on stars selected in stage 5.

@taldcroft
Copy link
Member

I haven't looked at recent stats, but I think that residuals above or near 5 arcsec are unusual, which is good because the OBC will be tossing the star at that point. Residuals above 3 arcsec are not unusual.

About warning on stage 5, not so keen. I'd much prefer checks that relate to some actual performance metric if possible. In the present implementation, that just brings in either a bright imposter (which will be warned) or a color=0.7 guide star (which will be warned). BTW, allow_0p7_color_star would be more informative than DoBminusVcheck and would have saved me having to check the code 😄.

However, this does point to a feature that I'm not sure we have discussed, which is putting the guide star stage into the starcheck report in the usual place. Not a requirement for 13.0, but it would good at some point. Also, it might be illuminating to add a histogram of guide star stage into the proseco guide 4.3.2 validation report. Have you ever looked at anything like that?

@jeanconn
Copy link
Contributor Author

Regarding the stage selected, I think I looked a bit, but still intended to do more (with more stage parameter review and optimization).

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Recent changes look good

@jeanconn jeanconn merged commit f56fbb4 into master Jan 17, 2019
@jeanconn jeanconn deleted the hot_pix branch January 17, 2019 15:56
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.

2 participants