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

Add track overlap check #380

Merged
merged 3 commits into from
Jan 3, 2022
Merged

Add track overlap check #380

merged 3 commits into from
Jan 3, 2022

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Dec 15, 2021

Description

Add track overlap check.

The PEA discards a readout window that overlaps with another window.  Review of the guide star tracking data (for the agasc supplement update) from the NOV2921A schedule shows that a guide star was frequently lost due to overlap with another guide star window. 
 
The existing spoiler checks in selection and review have prevented most overlapping stars from occurring, but the NOV2921A star selection shows that two selectable guide stars could be selected with the existing code within 35 arcsecs and have no starcheck warning.  This PR  adds a check that two tracked items (MON, FID, GUI, BOT) are not within 60 arcsecs.

Testing

Screen Shot 2021-12-19 at 9 39 06 PM

@jeanconn jeanconn changed the title Add track overlap check WIP: Add track overlap check Dec 15, 2021
@jeanconn jeanconn changed the title WIP: Add track overlap check Add track overlap check Dec 15, 2021
@taldcroft
Copy link
Member

Don't put permanent output into a tmp dir : Ran on NOV2921A, output https://icxc.cfa.harvard.edu/aspect/tmp/starcheck-pr380/starcheck.html#obsid45890.

@taldcroft
Copy link
Member

The description is too terse. Think of the PR description being something that external reviewers might quickly glance at, or more likely that we might view in 5 years.

@taldcroft
Copy link
Member

Can you copy/paste the relevant starcheck output into the description in case someone is viewing this without being on VPN?

@jeanconn
Copy link
Contributor Author

Ah. If we've decided we want the test outputs to be permanent that's fine.

@jeanconn
Copy link
Contributor Author

Or should the output just live in the PR? Maybe that is the most sustainable choice.

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 jeanconn requested a review from taldcroft December 20, 2021 16:28
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.

Looks good!

@jeanconn jeanconn merged commit cb4a866 into master Jan 3, 2022
@jeanconn jeanconn deleted the overlap branch January 3, 2022 18:27
@javierggt javierggt mentioned this pull request Feb 8, 2022
3 tasks
@javierggt javierggt mentioned this pull request Aug 3, 2022
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