Fix positioner angle range checks. #267
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This work:
- Handles the case of arbitrary positioner theta / phi
offsets and min / max.
- Adds a unit test which attempts to move every positioner
to a circle of spots halfway from the center to the patrol
radius. It also verifies intended failure in the case of
restricted phi range.
|
@tskisner wow! I'm going to compare the new run of the same observed tiles with the the ones passed to ICS on the nights of 14th and 15th and see if we can spot the rejected cases in difference between these two runs. Thanks much! |
|
Given that some of the online ics feedback was lost, is it possible to rerun the assignments and deduce what fibers should have been reported? Or is this not relevant / the case anymore. |
sbailey
approved these changes
May 8, 2020
|
I am running the code. Just give me one more hour to inspect the results. |
|
@forero , of course, no problem. |
forero
approved these changes
May 8, 2020
|
Ok, I'll update the changelog and then merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This work:
Handles the case of arbitrary positioner theta / phi
offsets and min / max.
Adds a unit test which attempts to move every positioner
to a circle of spots halfway from the center to the patrol
radius. It also verifies intended failure in the case of
restricted phi range.
Attached are plots of "unreachable targets" computed by taking uniform random RA / DEC points with a density of 50K per sq. deg. and computing the targets available for each positioner. Then these available targets were removed from the input sample and the remaining targets were plotted. Starting with the old legacy focalplane model (with equal 3mm arm lengths) you can see good coverage across the patrol radius. With the more realistic focalplane, you can see some unreachable targets near the center of each positioner and more around the edge of the patrol radius due to varying arm lengths. The final plot shows the commissioning focalplane with restricted phi range. Compare these plots to the ones in #266.
Note to @Srheft and @djschlegel , the current master branch did NOT apply the phi angle restriction correctly, which means that previous runs did not limit the phi range properly. This likely explains why you were assigning targets that were then rejected by the online system. This is now fixed, as you can see in the plots.
Closes #266.