rubin-dp0 / tutorial-notebooks

Tutorial Jupyter Notebooks for Data Preview 0, created and maintained by the Rubin Observatory Community Science Team.
Apache License 2.0
33 stars 17 forks source link

tickets/PREOPS-1203 #105

Closed MelissaGraham closed 1 year ago

MelissaGraham commented 1 year ago

Two new notebooks. 09a generates a user custom coadd. 09b runs source detection and measurement on the user custom coadd.

MelissaGraham commented 1 year ago

Use regex in 1.2 instead of temp.find(). E.g.,

import re
pattern = re.compile('^.deepCoadd.$')
for dst in sorted(registry.queryDatasetTypes(pattern)):
print(dst)
andy-slac commented 1 year ago

It seems that we really should allow it to take bind (which GraphBuilder does not accept either -- since we encourage use of bind in butler queries, shouldn't we be encouraging it in graph building using the APIs?

I made DM-36487 for that.

timj commented 1 year ago

@timj can confirm, but I think this is a case where it is recommended to use butler.getDirect() in lieu of butler.get().

Well spotted. Yes, that is a DatasetRef so getDirect is preferred.

jeffcarlin commented 1 year ago
  • Does my_sources not have a flux column that is already in nanojanskies? It might be good to comment that most of the source and object tables output by the stack are already in nanojanskies and don't need to be converted, or something along those lines.

I think Object tables have nJy fluxes, but Source tables always(?) have only instrumental fluxes.

MelissaGraham commented 1 year ago

@timj, @cmsaunders, @jeffcarlin, @leannep

Changes made for all as suggested, thank you -- except in a few cases I'm going to post individual comments about.

Versions of the executed notebooks have been committed, for now, so you can easily see changes/issues.

Once the remaining issues are resolved I'll commit unexecuted versions before merging.

MelissaGraham commented 1 year ago

@timj, @cmsaunders, I've got a new butler issue.

In NB 09a, S.3.2., the switch to the new simpleButler means that the output collection name is now using an automatic timestamp, e.g., "u/melissagraham/coadd_recreation_nb/Window1_coadd/20221015T011104Z". This is a good change. I have run 09a twice and so I have two versions of the custom coadd, the other having timestamp "20221015T014151Z". They're the same but let's suppose I made some change and that they're different.

In NB 09b, S.1.3., even when I define my collection using a specific timestamp, the butler knows about both. How do I define (and then check) which of the two new custom coadds is being selected by the butler.get()? I need some help under "NEED HELP HERE" in NB 09b. It's OK to edit the NB directly and commit if you know how to fix?

MelissaGraham commented 1 year ago

@cmsaunders, 09a, Section 2.2: I could not find anywhere 'deepCoadd_calexp' was being used, in either notebook. Am I missing it or was that a mistype?

MelissaGraham commented 1 year ago

@timj can confirm, but I think this is a case where it is recommended to use butler.getDirect() in lieu of butler.get().

Well spotted. Yes, that is a DatasetRef so getDirect is preferred.

@timj @cmsaunders, 09a, Section 4.2.1: Attempting to use getDirect instead of get returns "FileNotFoundError: Could not retrieve dataset deepCoadd@{band: 'i', skymap: 'DC2', tract: 4431, patch: 17}, sc=ExposureF]." So it still uses butler.get() in S.4.2.1.

MelissaGraham commented 1 year ago

@jeffcarlin, general comment: I did add more to the "scientific narrative" but it's not great, and maybe even more confusing. There is no actual DC2 SN at the adopted coordinates, no precursor events were even simulated for DC2 SNe, and this type of analysis should be done by stacking difference images, not stacking calexps. So I also added a bunch of caveats. Really, stacking images from a time window is just a clear and simple basis for this demo... I could change to stacking low-airmass or best-seeing but it's all set up to work as is, now... maybe in a future version.

cmsaunders commented 1 year ago

@cmsaunders, 09a, Section 2.2: I could not find anywhere 'deepCoadd_calexp' was being used, in either notebook. Am I missing it or was that a mistype?

Yeah, I don't see 'deepCoadd_calexp' anymore. Maybe it was changed in your last commit.

cmsaunders commented 1 year ago

@timj, @cmsaunders, I've got a new butler issue.

In NB 09a, S.3.2., the switch to the new simpleButler means that the output collection name is now using an automatic timestamp, e.g., "u/melissagraham/coadd_recreation_nb/Window1_coadd/20221015T011104Z". This is a good change. I have run 09a twice and so I have two versions of the custom coadd, the other having timestamp "20221015T014151Z". They're the same but let's suppose I made some change and that they're different.

In NB 09b, S.1.3., even when I define my collection using a specific timestamp, the butler knows about both. How do I define (and then check) which of the two new custom coadds is being selected by the butler.get()? I need some help under "NEED HELP HERE" in NB 09b. It's OK to edit the NB directly and commit if you know how to fix?

This is one of the reasons I would suggest having different collection names for different run configurations. @timj would know more advanced techniques, but by default the butler is going to just grab the latest run in a collection. I think of the timestamp as not really being 'user-facing', and is not meant to help you access different runs.

cmsaunders commented 1 year ago

@jeffcarlin, general comment: I did add more to the "scientific narrative" but it's not great, and maybe even more confusing. There is no actual DC2 SN at the adopted coordinates, no precursor events were even simulated for DC2 SNe, and this type of analysis should be done by stacking difference images, not stacking calexps. So I also added a bunch of caveats. Really, stacking images from a time window is just a clear and simple basis for this demo... I could change to stacking low-airmass or best-seeing but it's all set up to work as is, now... maybe in a future version.

I would just add that I know from talking to people at the LINCC workshop and in other forums that custom coadds is a super popular use case, so I don't think you need to worry too much about motivating the notebook.

MelissaGraham commented 1 year ago

Awesome, @cmsaunders, I've updated the recommended naming convention to be to use, e.g., "custom_coadd_window1_test1" and advising users to bookkeep and always rename their output collections (and not rely on timestamps, which I present as "FYI" only). That's working great.

MelissaGraham commented 1 year ago

Did a final check at data.lsst.cloud (had done all the dev work at data-int). All runs fine. Made a few final tweaks to some of the text, fixed some code standard violations, all good to go now.

Since this PR has been approved, am merging to main.