Fix for program ending during new field or same field setup.

[michaelJwilson]

As program ending was tested after the shutter was opened and ETC started, if a program
ended during new field or same field setup then negative exposures were generated by the reset

mjd_now  = mjd_program_end

I've added checks on program ending during setup after both new_tile and continue_same_tile, i.e. before ETC started, and an assertion on exposures > 0.

I think this was not specific to twilight, but affected each program change. However, adding more program changes and shorter exposures did exacerbate the problem.

[sbailey]

@dkirkby and @schlafly please take a look at this and comment.

We need to have a way to handle transitions between programs that doesn't generate negative exptimes (issue #65), but I think that logic should be owned in desisurvey.scheduler.Scheduler.next_tile() rather than by having the simulator watch for program transitions and make a choice to not call next_tile for awhile. i.e. the simulator should be simulating ICS, and ICS will be calling next_tile in these situations without waiting. So I think we need a different fix on the simulator side to accept whatever next_tile says, but not end up with negative exptimes.

[schlafly]

I agree that next_tile needs to be smarter about choosing which program to select a tile from near program boundaries. That said, I think next_tile should always return a tile if there are things that are observable; I definitely don't think that either the simulator or next_tile should see a program boundary coming and decide to sit around for a while. I think the easiest approach would be to select tiles in next_tile based not on the current program, but based on the program in 5 minutes (or something).

I wouldn't have guessed the simulator needed much in the way of special behavior around program boundaries. ETC.update() will stop a dark tile if the SNR^2 starts dropping, so I might have just went with 'finish the current tile, then ask next_tile for the next one.'

[michaelJwilson]

My minimal fix might not be optimal, but it solves a worse problem by design.

The branches are becoming very intertwined, it'd be nice to tidy this up to not have to merge separate threads every time.

[sbailey]

I agree that you've identified a genuine problem, but my core objection is that this PR fixes the problem in the simulator but not in the actual operations code, thus making the simulator a less realistic model of what would actually be observed (without further fixes on the desisurvey side).

The simulator should call next_tile just like ICS will (i.e. without any waiting or even knowledge about program boundaries), and any problems with what next_tile returns should be fixed on the desisurvey / next_tile side, rather than with a workaround on the simulator side to make it do what we wish the real system would do (but perhaps doesn't).

Could you try fixing this by removing any waiting logic here in surveysim, and then evaluating (and possibly contributing a fix) for what happens on the desisurvey / next_tile side?

[michaelJwilson]

Closing this given #65 remains as a record of the problem and the desired solution is orthogonal to this fix.