desihub / tutorials

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

Test tutorials on release 18.6 #17

Closed weaverba137 closed 5 years ago

weaverba137 commented 6 years ago

Post-release-18.6 clean up: make sure the tutorials work with release 18.6.

weaverba137 commented 6 years ago

@sbailey, in the Intro_to_DESI_spectra.ipynb notebook, some of those TARGETID = -1 fibers are showing up when the fibermap information is examined. Do we want to do anything special about those?

weaverba137 commented 6 years ago

Also from Intro_to_DESI_spectra.ipynb:

This highlights that redrock isn't doing a fantastic job of fitting quasars. It correctly identified some, though. And, of course, not everything targeted as a quasar will turn out to actually be a quasar (there are some contaminants), and in some cases redrock gets the redshift correct but still calls it a galaxy. Work in progress...

However, in 18.6, zs[zqsos]["SPECTYPE"] shows all "QSO". In other words, all the QSOs are classified as QSOs.

weaverba137 commented 6 years ago

For the survey-simulations.ipynb notebook: the notebook uses simulations found in surveysim2017/depth_0m/. Is that still the dataset to use, or is there something more recent?

weaverba137 commented 6 years ago

Similarly for dc17a-truth.ipynb, is that still the best, most up-to-date data set to use?

weaverba137 commented 6 years ago

In GFA_targets.ipynb, the function on_gfa_targets = desimodel.focalplane.get_gfa_targets(targets) is failing. I have saved the notebook in the test-18.6 branch with the full traceback.

dkirkby commented 6 years ago

Yes, surveysim2017/depth_0m/ is still the dataset to use.

dkirkby commented 6 years ago

I have saved the notebook in the test-18.6 branch with the full traceback.

I don't see any traceback here, but the schema for data/focalplane/gfa.ecsv changed in desimodel 0.9.6, so that is the most likely culprit.

I wonder if this notebook belongs in this repo where, in the README, we say:

This repository is for tutorials on simulating and working with DESI data. There are additional
more detailed tutorials within many of the DESI code repositories; tutorials here are generally for:
 - Topics that span individual repositories, e.g. working with data challenge outputs
 - Tutorials to be presented at a collaboration meeting where we want to decouple updates to the 
    tutorial itself from the desispec, desisim, etc. code versions that the tutorial is describing.

This notebook documents the development of one component of desimodel.focalplane (which is great) so would probably fit better in e.g., desimodel/docs/nb/.

weaverba137 commented 6 years ago

@dkirkby, when you say

I don't see any traceback here,

Is that because the notebook doesn't render at all?

weaverba137 commented 6 years ago

@dkirkby, never mind, looks like I forgot to push a commit.

weaverba137 commented 6 years ago

Hmm, and it looks like it still isn't displaying. @sbailey, what's your opinion on GFA_targets.ipynb being in the tutorials?

weaverba137 commented 6 years ago

@sbailey, just a ping. There are still a few open questions about the tutorial notebooks.

weaverba137 commented 6 years ago

@sbailey, I noticed you updated the minitest notebook to 18.7, however, we still haven't closed out the tutorials for 18.6. Can you review this issue?

sbailey commented 6 years ago

Noted, but it hasn't bubbled up to the top of my priority list yet. Will try to review and update tutorials this week.

sbailey commented 6 years ago

Thanks for testing these and itemizing the issues.

@sbailey, in the Intro_to_DESI_spectra.ipynb notebook, some of those TARGETID = -1 fibers are showing up when the fibermap information is examined. Do we want to do anything special about those?

I added a note about why those occur (unassigned or broken fibers).

Regarding the tutorial text "This highlights that redrock isn't doing a fantastic job of fitting quasars": that was a leftover from an earlier bug which has been fixed. I updated the tutorial to show the quasar results but not claim that many had failed since they are succeeding now.

In GFA_targets.ipynb, the function on_gfa_targets = desimodel.focalplane.get_gfa_targets(targets) is failing. I have saved the notebook in the test-18.6 branch with the full traceback.

This is a genuine bug in get_gfa_targets, which crashes when the input targets don't actually overlap any GFAs. I'll submit a fix, and we (I) should significantly update this tutorial for the 18.9 release. In the meantime, for 18.6 I've added a note explaining that this tutorial doesn't work.

Similarly for dc17a-truth.ipynb, is that still the best, most up-to-date data set to use?

The dataset is somewhat old, but the basic lessons of the tutorial as still valid so I've left as-is for now. At low priority we could update it to teach the same stuff based on a reference run.

sbailey commented 6 years ago

Tutorials updated and tagged for 18.6 release. Closing ticket.

weaverba137 commented 5 years ago

@sbailey, when you updated and tagged the tutorials, did you do that by merging the test-18.6 branch?