desihub / fiberassign

Fiber assignment code for DESI
BSD 3-Clause "New" or "Revised" License
7 stars 8 forks source link

Unassigned fibers should be placed at non-colliding locations #194

Open sbailey opened 5 years ago

sbailey commented 5 years ago

Current default for unassigned fibers (either no targets, or broken fibers) is to assign the positioner to its home location, but that can cause collisions for neighboring science target assignments.

It would be better to do a search for some non-colliding location and use that instead.

Best is to have sufficient SKY and BAD_SKY density so that every non-broken fiber is assigned to something. Broken fibers could be placed anywhere, but at least shouldn't cause an unnecessary collision.

tskisner commented 4 years ago

Revisiting this in Spring 2020. I think we now have sufficient density of sky, supp_sky, and bad_sky (safe) targets so that every positioner can be moved someplace. If not, we should improve that.

If we still need this logic in fiberassign, then I would suggest for each unassigned positioner:

  1. Start with the positioner arm fully closed (phi =~ 180)

  2. Sweep the theta angle from its minimum value across its range in steps

  3. At each theta step, check for collision. If none, stop.

This could be implemented as a small member function of fiberassign::Assignment and called from python just like the other methods.

sbailey commented 4 years ago

A point of clarification:

i.e. the first two cases we just have to live with wherever they are and not run into them; the second two cases give us flexibility for moving the positioner out of the way so that it doesn't unnecessarily collide with an otherwise good neighboring target.

tskisner commented 3 years ago

@sbailey, a question since I am implementing support for the case of broken fibers and stuck positioners. In these cases, the new desimodel code will provide the current estimated fixed positioner location (POS_P and POS_T). These can be used internally to fiberassign to avoid collisions. However, when writing the outputs, there is no column currently for writing the positioner angles that were used (we just write out the X/Y locations of the assigned target). For stuck positioners the target ID will be -1 or some dummy value. Would it be useful to write out the assigned positioner angles in the FIBERASSIGN hdu? If so, we should note that in #271.

sbailey commented 3 years ago

Thanks for working on this. I don't think that we need to write POS_P and POS_T quantities into the FIBERASSIGN HDU. i.e. we need them internally to avoid collisions, but I don't think we need them in the output.

tskisner commented 3 years ago

Ok, sounds good. Thanks for confirming.

tskisner commented 3 years ago

There has been some email discussion about parking unassigned positioners near a "fully folded" position. Not only does this place the location near the center where small discrepancies with the online system may reject the location, it is also not ideal to have a potentially large move.

For working positioners, we do not track the current location (so can't create a minimal move that is non-colliding), however we should try fixing the phi angle to something more like 120 degrees (not 180) before sweeping theta to find a non-colliding location.

deisenstein commented 3 years ago

Choosing a value around 120 seems reasonable to me. Thanks!

On Wed, Apr 21, 2021 at 10:19 AM Theodore Kisner @.***> wrote:

There has been some email discussion about parking unassigned positioners near a "fully folded" position. Not only does this place the location near the center where small discrepancies with the online system may reject the location, it is also not ideal to have a potentially large move.

For working positioners, we do not track the current location (so can't create a minimal move that is non-colliding), however we should try fixing the phi angle to something more like 120 degrees (not 180) before sweeping theta to find a non-colliding location.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/desihub/fiberassign/issues/194#issuecomment-824098922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPQRGU2PSPBH7UAB2R6RZDTJ3NFZANCNFSM4G7TG55Q .

tskisner commented 3 years ago

Quoting an email from Joe on 4/21/21:

Regarding "parking", we typically put positioners at pos_T, posP = 0, 150 deg as our standard parking position. That would be a fine convention for fiber assign to adopt. 150 is a better value than 120. At 120, positioners with large magnitude offset_p might still be peeking out of the retracted zone.

dstndstn commented 3 years ago

150 deg was implemented in #340