desihub / desisurvey

Code for desi survey planning and implementation
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

exposure time correction factor for bright time #102

Closed changhoonhahn closed 4 years ago

changhoonhahn commented 5 years ago

exposure time correction factor for bright time implemented in desisurvey.etc.bright_exposure_factor.

surveyinit script adjusted to include sun positions.

desisurvey.scheduler.Scheduler now includes desisurvey.etc.bright_exposure_factor in the self.exposure_factor calculation.

sbailey commented 5 years ago

Thanks @changhoonhahn . I'm sure this has been discussed on BGS telecons or somewhere, but could you point us at some documentation comparing this model to the old model, why it is better, what impact it has on the big picture (e.g. changing predicted survey margin), etc.?

At a technical level, we need to find a replacement for the pickle file, since that format is not guaranteed to to work from one version of python to another and is completely unusable outside of python if someone wanted to use a different language to study this. It would also be preferable to convert the hdf5 file into a FITS file so that we don't have to bring in a new hdf5 dependency in desisurvey. Could you describe the structure of these files and how readily they could be mapped to FITS or JSON?

michaelJwilson commented 5 years ago

We'll need a version that predicts the ETC at 7200A aswell? Given this will seemingly determine the program time.

Is there an error bar associated to the emulator output? It'd be useful to know when you should be calling the actual model, or where the data sampling is sparse.

schlafly commented 5 years ago

I wanted to restart the conversation on this PR. It's clear that we should be using a much more sophisticated model for the sky, as this PR puts into place. Without this, the bright time simulations are seriously limited.

That said, I'd like to understand better how we want to incorporate the sky into the ETC. Currently, this PR adjusts the amount of time each tile takes by tweaking the exposure factor returned by scheduler.next_tile. I'd argue that the only exposure factor next_tile should return should be the constant MW E(B-V) portion, and that everything else should be handled by the ETC, through improving the sky values in the survey simulations here: https://github.com/desihub/surveysim/blob/12067c90b88337e0e2b32c93d975d5e4c4cf65fc/py/surveysim/nightops.py#L168 In this vein, it would be good if this new sky model had a convenient function for the sky background, rather than just the exposure factors.

Currently next_tile also handles the airmass portion of the exposure factor, though really, the airmass just changes the FWHM, transparency, and sky background, and the ETC should be tracking all of these, so it strikes me that the airmass_exposure_factor shouldn't really be something next_tile uses.

Looking through the ETC now, do we really want separate "moon" and "bright" exposure factors? I could imagine the improved sky model introduced by this PR replacing the moon_exposure_factor (i.e., I don't think we want a separate "bright_exposure_factor" and "moon_exposure_factor). I couldn't immediately find any places where moon_exposure_factor is actually used, so maybe this isn't a big concern.

michaelJwilson commented 5 years ago

Copying this across:

I'd argue that the only exposure factor next_tile should return should be the constant MW E(B-V) portion, and that everything else should be handled by the ETC, through improving the sky values in the survey simulations here:

In picking the tile, it seems obvious that an expected exposure time should be accounted for, at least as a (greedy) possibility? Then, to save recalculating, it seems fine to pass this to the tracking of an exposure. Once you slew to the new position and can then get the actual sky background, then you can scale more accurately with the measurement, as for seeing and transparency.

Note that an important aspect of this is twilight, when the raising sun should shut off the exposure in nightops - e.g. I have a branch that tidies up the sky definition in night ops to account for the changes relative to the exposure prediction calculated in next_tile. Master is halfway to implementing this.

Looking through the ETC now, do we really want separate "moon" and "bright" exposure factors? I think it makes sense to have separate moon and sun factors? Useful for cross-checking.

p.s. I'm not a big fan of the emulator aspect - it's likely to break down exactly where we don't want it to. I have an analytical fit that recovers Chang's actual ETC, but the band definition etc. may need juggled.

schlafly commented 5 years ago

Once you slew to the new position and can then get the actual sky background, then you can scale more accurately with the measurement, as for seeing and transparency.

Right, this is the role of the ETC.

In picking the tile, it seems obvious that an expected exposure time should be accounted for, at least as a (greedy) possibility? Then, to save recalculating, it seems fine to pass this to the tracking of an exposure.

Honestly, I haven't yet seen much evidence that the greedy selection buys a lot. But yes, for the purposes of a greedy selection it makes sense for the NTS to estimate the exposure time factor. But that's a matter of next_tile's internal algorithm. What I don't like is information being propagated back into surveysim/nightops to determine how much exposure time a tile actually needs. After a tile has been picked, the ETC should figure out how long to observe on its own, and not rely on logic in the tile picker.

michaelJwilson commented 5 years ago

What I don't like is information being propagated back into surveysim/nightops to determine how much exposure time a tile actually needs. After a tile has been picked, the ETC should figure out how long to observe on its own, and not rely on logic in the tile picker.

I think you want to simulate the sky background etc. accurately and then evaluate the time for whatever tile gets passed, as for seeing. This makes sense and the code is already halfway there on this. It just strikes me that it might be a small reshuffle and personally I'd probably prioritise other deficiencies first.

Bottom line is that there's just a need for an overall game plan.