Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project/hsc re run #86

Merged
merged 18 commits into from
Sep 28, 2018
Merged

Project/hsc re run #86

merged 18 commits into from
Sep 28, 2018

Conversation

jtmyles
Copy link
Contributor

@jtmyles jtmyles commented Aug 9, 2018

This preliminary commit includes some code directly from the source of the command-line routines for part I (using the butler). @drphilmarshall could you review this preliminary commit? One question that I think we should address at this stage is whether the code from the command-line routines used here gives the right level of insight into how the stack instantiates a butler. For example, I'm unsure of whether users will want to see more of how the ingest script works. I'd appreciate all comments anyone has -- thanks!

Copy link
Contributor

@drphilmarshall drphilmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the level of detail needed: I think you should mainly include what you need, to understand what is going on, and then try to give references in case the reader wants to find out more. If you want to leave explanation of the ingest script to another notebook, just say that its beyond the scope of this notebook and move on. If you have an idea for a separate tutorial that explains the ingest script, you could issue it as a possible (ie unassigned) project.

"# HSC Re-Run: Making Forced Photometry Light Curves from Scratch\n",
"\n",
"<br>Owner: **Justin Myles** ([@jtmyles](https://github.com/LSSTScienceCollaborations/StackClub/issues/new?body=@jtmyles))\n",
"<br>Last Verified to Run: **N/A -- in development**\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even in development we need the notebooks to run at all times - in fact you need the notebook to run at all time, so you can test that the code you are writing works!

"outputs": [],
"source": [
"!eups list lsst_distrib\n",
"datarepo = \"/home/jmyles/repositories/ci_hsc/\"\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this runnable by anyone, we'll need to use $USER instead of jtmykes, and mkdir -p to make a new director to use.

ImageProcessing/Re-RunHSC.ipynb Show resolved Hide resolved
"metadata": {},
"outputs": [],
"source": [
"with open(datadir + \"_mapper\", \"w\") as f:\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd set the mapper file name in a variable here, and then call open separately - that way we'll know what file to look for in teh folder.

"outputs": [],
"source": [
"# ingest script\n",
"!ingestImages.py /home/jmyles/DATA /home/jmyles/repositories/ci_hsc/raw/*.fits --mode=link"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, you should explain why uou are just running the command line task, not unpacking it. I'd do this even if you plan on changing things later: as well as the notebook being able to "just run" , it also needs to be able to be "just read", by anyone who comes across this repo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to say this is that the documentation in the code is part of the code.

"metadata": {},
"outputs": [],
"source": [
"#!installTransmissionCurves.py /home/jmyles/DATA\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cell makes me realize, it would be really nice to have a link to the source of each command line task in the markdown cell above the cell where you run or unpack that task, for reference.

"metadata": {},
"outputs": [],
"source": [
"# ingest calibration images into Butler repo\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When explaining the calib files, I wonder if it's worth including a few ls commands to show the user what is available in teh data repo. Same for the science images. Or, you could refer to the Basics/DataInventory tutorial and have the user go look at that instead.

@jtmyles
Copy link
Contributor Author

jtmyles commented Sep 6, 2018

The latest commit to this branch now includes a bash script that should 'just run' the tutorial at pipelines.lsst.io. The final part of the tutorial is done in part 6 of the Re-RunHSC.ipynb notebook.

Copy link
Contributor

@drphilmarshall drphilmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two suggestions: extend the script to include the forcedPhotCcd.py task, and have the notebook print out the script.

echo "run forcedphot on coadds"
forcedPhotCoadd.py $DATADIR --rerun coaddPhot:coaddForcedPhot --id filter=HSC-R
forcedPhotCoadd.py $DATADIR --rerun coaddForcedPhot --id filter=HSC-I

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good - now we need step 'V. F. Run forcedphot on visit images' and call forcedPhotCcd.py --rerun coaddForcedPhot:ccdForcedPhot to generate the forcedSource (?) catalog(s). And then you're done!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although: you should also find out how to merge such measurements, or alternatively, whether you just query the catalogs separately and construct a multi-filter light curve yourself. The LSST DESC's Monitor package might be useful for this.

ImageProcessing/Re-RunHSC.ipynb Show resolved Hide resolved
@drphilmarshall drphilmarshall merged commit c5e5c2b into master Sep 28, 2018
@drphilmarshall drphilmarshall mentioned this pull request Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants