LSSTDESC / DC2-production

Configuration, production, validation specifications and tools for the DC2 Data Set.
BSD 3-Clause "New" or "Revised" License
11 stars 7 forks source link

Review of DC2_Postage_Stamps.ipynb #204

Closed kadrlica closed 6 years ago

kadrlica commented 6 years ago

This is an issue to track feedback on DC2_Postage_Stamps.ipynb by @wmwv. Some feedback that was specifically requested:

kadrlica commented 6 years ago

I was able to get this notebook to run all cells successfully after setting up the desc kernels as documented here: https://confluence.slac.stanford.edu/display/LSSTDESC/Using+Jupyter-dev+at+NERSC

kadrlica commented 6 years ago

In cell 3 when cutout_coadd_spherepoint is defined, is the skymap=None generalizable for datasetType other than 'deepCoadd'. Specifically, I'm suspicious of the lines below, but not familiar enough with the skymap object returned by the butler.

 if skymap is None:
        skymap = butler.get("deepCoadd_skyMap")
wmwv commented 6 years ago

I agree that it should be

skymap = butler.get("%s_skyMap" % datasetType)

This doesn't really matter given that we dohn't have any other types of coadds in Run 1.1p or planned for DC2, but I agree with the coding question that given that the surrounding function accepts a datasetType why don't we use it here?

kadrlica commented 6 years ago

Ok, I'll commit that change in #205.

@wmwv here are a few more thoughts about the notebook from going through the first half. I'll try to add some of these to my PR.

kadrlica commented 6 years ago

A few other very minor comments on the notebook.

wmwv commented 6 years ago
kadrlica commented 6 years ago

Cell 16: agreed, but it's also cutting off your explanation that the doc is hard to understand :)

wmwv commented 6 years ago

Ah, I didn't pick up on that. The line wraps correctly in my version.

Hmmm... it wraps correctly in Jupyter[Hub] but not in JupyterLab. I'll keep this as a small point to fix someday.

bechtol commented 6 years ago

Its too bad that the notebook backend isn't yet available in JupyterLab because there is a real value in the interactivity offered.

Anyhow, when running directly in Jupyter, I wanted to re-run the # Plot just one i = 5 first = id_ra_dec[i] ra, dec = first['RA'], first['DEC'] cutout = make_cutout_image(butler, ra, dec, filter=filter, label="Object ID: %d" % id_ra_dec[i]['ID']) cell to see a different postage stamp, but the plot does not seem to be regenerated. Note that this only happens when using the notebook backend directly with Jupyter. It seems that the postage stamp is being plotting in another figure cell that was run previously. I think behavior could be fixed by adding a line to create a figure in the make_cutout_image function. Maybe I should make a pull request.

RobertLuptonTheGood commented 6 years ago

Sorry to repeat myself, but at NCSA we support %matplotlib ipympl which works with JupyterLab.

bechtol commented 6 years ago

Thanks @RobertLuptonTheGood , I'll try that backend when lspdev is back up. I was running the DC2 tutorial notebooks at NERSC.