Implement include/exclude_ids in guide star selection

[jeanconn]

Implement include/exclude_ids in guide star selection

[jeanconn]

This was the most direct hack to accomplish the behavior. I didn't have in mind this use case when doing the original implementation, so it could really use a rewrite to have functions to manipulate the list of candidate stars.

[taldcroft]

I may be missing some logic in the code, but here goes:

It seems at first glance that you can use almost exactly the code here:
https://github.com/sot/proseco/blob/master/proseco/acq.py#L405

And put it here:
https://github.com/sot/proseco/blob/master/proseco/guide.py#L332

Basically do exactly all the previous initial guide candidate selection, and at the very end if an included guide star is not in the cand_guide table then do the "bare minimum" validation of the included star and toss it into the candidates list by adding the row into the table. This guarantees the star can be selected. Then you still need the new code to actually select it at stage 0.

Likewise after putting in the include stars then check if any cand_guide star is in the exclude list and delete the row in the cand_guides table (and put in the usual logging). This will prevent it from being selected. (BTW, in acq I think I did not take this approach in order to keep the star in the cand_acqs table for visibility since there is no reject logging like guide. In the case of acq the subsequent exclusion was just a couple of lines of code).

At any point earlier in the processing you can just check set(self.include_ids) & set(self.exclude_ids) for an overlap and throw an exception. This separates out validation from processing to some extent.

If I haven't misunderstood what's happening, I think doing it like this will be a more confined change.

[taldcroft]

A corollary to the recommendation of using ok for boolean masks is that this should only be done for code that is immediately adjacent. So move this statement down to where it is used.

[jeanconn]

Thanks @taldcroft .

• Forgot to mark as WIP
• I think you are basically right about implementation
• My pre-this-hack version was a bit more like what you are describing. But since I annotate the larger set of candidates with information about how they behaved in the checks somewhat as a side effect of running those checks, my brain was not happy with not having values for those checks for a forced candidate. Thus, this first full implementation carried along the forced candidates and put them through the checks (to have more complete information for the report). However, for the special case of forced stars it probably makes more sense to just handle debug and warnings from the starcheck side.

[jeanconn]

I've just updated this to be more consistent with @taldcroft 's vision. There are still compromises in some code that should be addressed in guide star selection, but this is at least a more lean change.

[jeanconn]

I ended up not adding back in code to check that include_ids aren't in exclude_ids.

[taldcroft]

Looks good!

[taldcroft]

Related to the discussion in #178, maybe revert this test and then fix the expected catalog to show both guide and acq exclusion of ..3544?

[jeanconn]

OK. I was thinking that the test coverage of the supported interface was actually better this way ( test_aca_acqs_include_exclude seems to include and exclude the same stars for guide and acq, only via the separate kwargs).

[taldcroft]

OK fair enough.