desihub / tutorials

DESI tutorials
BSD 3-Clause "New" or "Revised" License
43 stars 17 forks source link

update survey-simulations tutorial to work with latest desisurvey/surveysim #23

Closed sbailey closed 5 years ago

sbailey commented 5 years ago

The current survey-simulations tutorial works with the old desisurvey/surveysim in the 18.11 software release, but not with the newer faster better desisurvey/surveysim in 18.12 and later. Please update the tutorial to work with the latest release.

sbailey commented 5 years ago

@dkirkby could you update this surveysim tutorial prior to the collaboration meeting? We'd like to have it working by the tutorial session on Monday July 8, which leads into the survey strategy session on July 9 and the hack day on July 12.

If you can't make the updates yourself, could you recruit someone who can? Thanks.

sbailey commented 5 years ago

I just chatted with @schlafly who agree to take a look at this. Thanks.

schlafly commented 5 years ago

I just went through this, and the tutorial mostly runs through how to use the now deprecated 'progress' module. @dkirkby, how do you envision the tutorial being updated? I could imagine:

dkirkby commented 5 years ago

I will take a look at this now...

dkirkby commented 5 years ago

I think the most useful material for next week's tutorial session would be some examples using the 100 weather realizations in $DESI_ROOT/datachallenge/surveysim2018/weather/, both individually and as an ensemble.

Each realization contains exposures.fits and stats.fits (but unfortunately no weather.fits), so this means:

Thanks for taking a look at this @schlafly!

schlafly commented 5 years ago

Thanks David. I took a look at this. Here's what I put together https://github.com/desihub/tutorials/blob/surveysim192/survey-simulations.ipynb

I still need to remove some redundancy between what you had already done and what I added, but at least everything works now.

Some minor comments...

I didn't use the wrapper class here; it doesn't really have any useful access methods, and later accessing the relevant fields through _exposures and _tiledata felt a bit odd?

  • Loop over the 100 weather realizations to study variability in, e.g., number of DARK tiles completed after year 4.

This was slightly more involved than I wanted, given the absence of copy_range, but it worked out all right. But I just queried completed exposures falling into an MJD range out of the exposures table for each run, and that looks like it worked fine.

dkirkby commented 5 years ago

Looks good! Some minor suggestions below:

The outputs from a survey simulation are two FITS files, one organized by tile and the other organized by exposure.

There is now one file with separate HDUs for per-tile and per-exp info (as your code shows).

tiledat = desimodel.io.load_tiles()

I recommend using tiledat = desisurvey.tiles.get_tiles() (docs) instead since this ensures you use the same tiles files consistently (and provides some utilities like airmass). Replace tiledat['ra'] with tiledat.tileRA, etc (attrs are tileID, passnum, tileRA, tileDEC, dust_factor).

I see that you use desisurvey.tiles.Tiles() later, but desisurvey.tiles.get_tiles() has the advantage of only reading the file into memory once when called several times.

Append semicolon (or plot.show()) to plot_sky_passes() lines to avoid the distracting return values.

schlafly commented 5 years ago

Looks good! Some minor suggestions below:

Thanks David. These are all in. I just opened a PR to merge this, and put your name on it. Thanks!

weaverba137 commented 5 years ago

Fixed by #28