sot / proseco

Probabilistic star evaluation and catalog optimization
https://sot.github.io/proseco
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Use spoiler FM offset constraint in guide selection #369

Closed taldcroft closed 2 years ago

taldcroft commented 2 years ago

Description

Based on discussions in Slack (in:aspect-team pre-review of obsid 23861 in the OCT0421 prelim), this PR implements a more fine-grained approach to spoiler checks in guide star selection. This uses the curves derived long ago by Dave Morris in the right plot in Fig 2 of https://cxc.harvard.edu/mta/ASPECT/Papers/radial_spoilers.ps to establish delta-mag limits for allowed offsets of 0.05 arcsec and 0.5 arcsec. These are allowed at stages 4 and 5, respectively.

The 0.0 arcsec offset (stages 1-3) is the same as the legacy SAUSAGE "direct catalog check" of 9.0 mag - 0.5 * dist (pix).

The curves themselves were read off the published contour plots using an online utility https://apps.automeris.io/wpd/.

ToDo:

Testing

Functional testing

Selected stars for obsid 23861 (the original obsid from the slack discussion) and the expected star 257165856 with a 14.6 mag (non-) spoiler is indeed selected. Note that if the 0.5 offset is moved to stage 6 then it is NOT selected since a fainter star gets selected first in stage 6.

jeanconn commented 2 years ago

I think we didn't do this before Tom because you had concerns about the spoilers on the background pixels.

taldcroft commented 2 years ago

I think we didn't do this before Tom because you had concerns about the spoilers on the background pixels.

I thought check_spoil_contrib is supposed to take care of that.

    Check that there are no spoiler stars contributing more than a fraction
    of the candidate star to the candidate star's 8x8 pixel region or more than bgthresh
    to any of the background pixels.
jeanconn commented 2 years ago

My recollection was that check_spoil_contrib was added because the concern about spoilers on the background pixels was very recent / forward in your thinking but I don't recall the check or its tolerances being evaluated much ... to then allow the "line" check to get relaxed / removed / updated.

taldcroft commented 2 years ago

check_spoil_contrib was in the first 1.0. release of proseco in Sep 2018, so not really recent. But maybe I'm not understanding your statement.

jeanconn commented 2 years ago

I meant "added" in the original development. The development process was largely adding checks (based on some additional knowledge after SAUSAGE development and some more conservatism with the transition to proseco for guide selection), and then I wasn't sure about using, say, the tracking alg in annie to see if the ad hoc checks corresponded to roughly to what looks useful when it comes to a star getting .. perturbed by a spoiler. But if the check_spoil_contrib threshold(s) seem reasonable to you that's fine.

taldcroft commented 2 years ago

Since we are relying on check_spoiler_contrib more with this change, I'll do a little more poking at that.

taldcroft commented 2 years ago

the concern about spoilers on the background pixels was very recent / forward in your thinking

Though I still don't know what you mean about "recent / forward" here. If you recall something I said in the last year or so on the topic please let me know, because I have forgotten.

taldcroft commented 2 years ago

Speaking of forward, we need to consider this in the context of dynamic background, where the spoiler star will pollute the background map. Hrrmmmm, that might actually be more of a problem.

jeanconn commented 2 years ago

In this case I was speaking about "recent at the time of making the guide star checks in proseco", so that's vintage. I think the only super-recent comment you'd had on this was (slack link removed... to a comment from Tom on July 26 2021 with " I'm a little sketched about the 12.6 mag spoiler getting into the background readout and going non-linear. I'm not really sure, just feeling less confident today." ) in the context of overriding the proseco guide star selection.

jeanconn commented 2 years ago

Assuming I can just link into slack without security issues. Anyway, yes, I wasn't sure if we wanted to change any checks or make any optimizations for dynamic background, but was thinking about that being more in the weeds (but yes, not just the corner pixels are a problem for dynamic background).

taldcroft commented 2 years ago

Assuming I can just link into slack without security issues.

(I've been providing specific search terms to get back to old conversations). I don't have any specific reason to think a link is not secure, but I also don't know about potential vulnerabilities.

That case in question is probably different since it was a force include that also violates ASPQ1. Don't know if it violates check_spoiler_contrib and even the new spoiler centroid offset check (probably close).

jeanconn commented 2 years ago

I removed my slack link though it would probably be good to know. And yes, the thoughts you had about how willing you were to override the proseco selection for that particular case of a spoiler star in the corner and would not have satisfied the ASPQ1 (which was 33 for that star and only relaxes as far as 20 in the guide star stages).

But looking back over https://github.com/sot/ska-projects/issues/55 I've gotten a bit confused, so will read the paper to see how the data you're bringing in for this updated check differs from ASPQ1 itself.

taldcroft commented 2 years ago

One obvious difference is that the new test uses supplement mags. But you're right that they may be conceptually very similar.

jeanconn commented 2 years ago

Right. Which also brings up the question of how to evaluate how much value could be added by recalculating ASPQ1 and including it in the supplement.

taldcroft commented 2 years ago

This PR is probably not going to happen given the issues noted with dyn bgd. Closing.