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

Check positioner motion when constructing TargetsAvailable. #264

Merged
merged 1 commit into from Apr 27, 2020

Conversation

tskisner
Copy link
Member

The TargetsAvailable class stores the initial list of targets
to consider for each positioner. This is before any checks for
collisions or the remaining obs for each target. Previously,
the check for whether these targets are physically reachable was
done at the same time as collision checks during assignment.
This work moves that "physically reachable" check to the earlier
step when computing TargetsAvailable. So now the available targets
that are written to the output include this constraint.

Closes #263.

The TargetsAvailable class stores the initial list of targets
to consider for each positioner.  This is before any checks for
collisions or the remaining obs for each target.  Previously,
the check for whether these targets are physically reachable was
done at the same time as collision checks during assignment.
This work moves that "physically reachable" check to the earlier
step when computing TargetsAvailable.  So now the available targets
that are written to the output include this constraint.

Closes #263.
@tskisner tskisner self-assigned this Apr 25, 2020
@michaelJwilson
Copy link
Contributor

It's clear what you're trying to do so it largely looks good to me. Maybe Stephen can still verify.
Are there any potential 'gotchas' in the definition of bad_xy here? Is it set early with all required hardware constraints etc? Timeline wise, it'd be great to get this sorted by e.g. tomorrow or Wed.
Although I can work off the PR if necessary.

@sbailey
Copy link
Contributor

sbailey commented Apr 27, 2020

A clarification note since it wasn't clear to me: previously FAVAIL / POTENTIAL_ASSIGNMENTS HDUs included targets that were close enough to consider even if upon further inspection it turned out that they weren't physically reachable. This PR updates that so that only physically reachable targets are included (which are the ones we care about for the HDU).

@sbailey sbailey merged commit b3cfe94 into master Apr 27, 2020
@sbailey sbailey deleted the issue_263 branch April 27, 2020 22:09
@michaelJwilson
Copy link
Contributor

That's correct. The decision of physically reachable has now been moved before the FAVAIL HDU is defined and therefore included in that decision. Before, it was a decision based solely on the arm lengths. So, e.g. for the reduced reach in SV data (that was implemented by theta, phi restrictions) objects that were in FAVAIL were very much not reachable.

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.

When computing TargetsAvailable check for actual positioning ability
3 participants