desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

Better functionality for fiber-based ToOs and ledger overrides #795

Closed geordie666 closed 2 years ago

geordie666 commented 2 years ago

This PR updates how ToOs with TOO_TYPE=="FIBER" are handled. It also adds some new functionality for the override ledgers that we use to modify targets that are already in the MTL ledgers.

Updates to the ToO functionality include:

Updates to the override-ledger functionality include:

geordie666 commented 2 years ago

@araichoor is checking the updates to the ToO mechanism from the fiberassign perspective. I'll likely merge this and cut a new desitarget tag once Anand is satisfied that everything gels.

coveralls commented 2 years ago

Coverage Status

Coverage decreased (-0.07%) to 56.396% when pulling 1d7d6632ee0c9bf863c6c46cf7f8506b85049dc3 on ADM-ToO-over into 1c7edc091ba7b8c628914826abcd5ee9c7a8bf24 on master.

araichoor commented 2 years ago

I ve coded up the twin PR in fiberassign, which works fine for the main survey tiles.

one question though, when designing ToO-dedicated tile (maybe deprecated with the tertiary programs):

in the current fiberassign code, for a ToO-dedicated tile, will read ToO.ecsv and select both TOO_TYPE="TILE" and "FIBER" targets: https://github.com/desihub/fiberassign/blob/854abc4d8c09250bb88855ee55be5d89a25fef38/py/fiberassign/fba_launch_io.py#L1637-L1638 with the current scheme, for that case, the code will read the ToO.ecsv file, and hence just grab the TOO_TYPE="TILE" targets. (Adam, actually that the initial reason of my question on slack if ToO.ecsv would contain both TOO_TYPE="TILE" and "FIBER" targets; but I then forgot...)

maybe it s fine, but I just wanted to mention.

geordie666 commented 2 years ago

I think the case you describe is probably fine, Anand. I suspect we'll want to use the tertiary process in the future instead of TOO_TYPE="TILE". If not, then the ToO folks would just have to supply their targets as "TILE" targets instead of "FIBER" targets.

I'm going to merge this soon and make a new tag of desitarget. I have lots of meetings today, but I'll try to have a tag ready as soon as possible.

araichoor commented 2 years ago

ok perfect then, thanks!