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

Add a tutorial for joint 1d/3d analysis #2505

Merged
merged 1 commit into from Nov 6, 2019

Conversation

@QRemy
Copy link
Contributor

QRemy commented Nov 1, 2019

This tutorial illustrates how to run a joint analysis with different datasets. We look at the gamma-ray emission from the Crab nebula between 10 GeV and 100 TeV. The spectral parameters are optimized by combining a 3D analysis of Fermi-LAT data, a ON/OFF spectral analysis of HESS data, and flux points from HAWC.

@QRemy QRemy requested a review from adonath Nov 1, 2019
@QRemy QRemy added the docs label Nov 1, 2019
Copy link
Contributor

registerrier left a comment

Thanks @QRemy ! This looks great.
I have included some suggestions and some general comments in the notebook.
I guess the execution time might a bit long at the moment. Maybe reducing the Fermi map size would help.

tutorials/joint_1d_3d_analysis.ipynb Outdated Show resolved Hide resolved
"cell_type": "markdown",
"metadata": {},
"source": [
"We use the events associated to 3FHL catalog in order to a produce counts cube. We select a 10-by-8 degree region centered on the Crab in Galactic coordinates, and photon energies between 10 GeV and 2 TeV. "

This comment has been minimized.

Copy link
@registerrier

registerrier Nov 2, 2019

Contributor

I would explain that this is not performing a real data reduction of Fermi data but simply taking reduced products from the 3FHL catalog to create our own dataset.

Also a 10x8 degree map seems large for data above 10 GeV no?

This comment has been minimized.

Copy link
@QRemy

QRemy Nov 4, 2019

Author Contributor

yes, reduced to 5x4

tutorials/joint_1d_3d_analysis.ipynb Outdated Show resolved Hide resolved
"background_total.sum_over_axes().smooth(2).plot(\n",
" stretch=\"log\", vmax=50, cmap=\"nipy_spectral\"\n",
");"

This comment has been minimized.

Copy link
@registerrier

registerrier Nov 2, 2019

Contributor

I think that this cell shows that we need a helper function to create a BackgroundModel from a SkyDiffuseCube. I don't think MapEvaluator should be something a typical user has to work with.

This comment has been minimized.

Copy link
@QRemy

QRemy Nov 4, 2019

Author Contributor

Maybe, for now I just moved all the datasets preparation steps in a separate script and only read pre-made files in the analysis script.

"background_total.sum_over_axes().smooth(2).plot(\n",
" stretch=\"log\", vmax=50, cmap=\"nipy_spectral\"\n",
");"

This comment has been minimized.

Copy link
@registerrier

registerrier Nov 2, 2019

Contributor

For this part, using the high level interface might be nice.

This comment has been minimized.

Copy link
@QRemy

QRemy Nov 4, 2019

Author Contributor

Same comment that above.

"background_total.sum_over_axes().smooth(2).plot(\n",
" stretch=\"log\", vmax=50, cmap=\"nipy_spectral\"\n",
");"

This comment has been minimized.

Copy link
@registerrier

registerrier Nov 2, 2019

Contributor

Also this part should not appear here. We should ship an exclusion mask for the crab in the data and provide an howto to explain how to create one from a catalog.

tutorials/joint_1d_3d_analysis.ipynb Outdated Show resolved Hide resolved
"background_total.sum_over_axes().smooth(2).plot(\n",
" stretch=\"log\", vmax=50, cmap=\"nipy_spectral\"\n",
");"

This comment has been minimized.

Copy link
@registerrier

registerrier Nov 2, 2019

Contributor

Is this part useful here? You don't use the covariance matrix here right?

This comment has been minimized.

Copy link
@QRemy

QRemy Nov 4, 2019

Author Contributor

It's used by crab_spec.plot_error().

tutorials/joint_1d_3d_analysis.ipynb Show resolved Hide resolved
tutorials/joint_1d_3d_analysis.ipynb Show resolved Hide resolved
@cdeil cdeil added this to To Do in DOCUMENTATION via automation Nov 3, 2019
@cdeil cdeil added this to the 0.15 milestone Nov 3, 2019
@cdeil

This comment has been minimized.

Copy link
Member

cdeil commented Nov 3, 2019

@QRemy - Thanks!

Please list the notebook in tutorials/notebooks.yaml, so that it gets executed and tested as part of continuous integration.

Also, please add the notebook in the toctree and as a bullet point in docs/tutorials.rst so that it shows up in the Gammapy docs. Test locally with make docs-all and make docs-show that the tutorial notebook does show up, and that it looks OK.

Please try to add more level 2 section headings, and keep them very short. Example: https://docs.gammapy.org/dev/notebooks/maps.html
The reason is that the section headings are part of the navigation sidebar, so very long names don't fit there. And they are anchors one can link to when sharing URLs to show colleagues how something works, or also we can link to sections from other parts of the Gammapy docs (e.g. the HOWTO we're starting). So if you have very long sections, it's hard to link to or tell people where something is.

I didn't read through the tutorial. @registerrier already made some suggestions, and probably @adonath wants to review this as well. So I'll just leave some very general suggestions:

  1. Suggest to only polish this for ~ 1 day for now and then go back to improving datasets code. (and then come back to this tutorial and other docs in the coming weeks)
  2. Try to shorten this. Overall the notebook is very long and I think ~ 70% of this tutorial does data reduction and duplicates other tutorials? The main focus of this tutorial should be on datasets serialisation / loading, and multi-dataset fitting.
  3. Does it run fast? Aim for 10 - 20 sec to execute the tutorial. If it takes much longer, do something to make it run faster (e.g. smaller FOV or less data, or better fit start parameters). Please add a %%timeit to the one or few cells that take the longest to execute - IMO it's nice for readers to see where usually the heavy computations will happen.
  4. Please never commit Jupyter notebook output cells. They usually contain images and are a few MB per notebook, which if we put this in our version history, with re-execution and many notebooks quickly snowballs to a very large Gammapy code repository which we want to avoid. So please use make polish to clean the output cells and amend the commit here, to actually not have the images in the version history. If you want to share a rendered notebook to make review easier (which is very useful) - post it in a https://gist.github.com/ paste the Gist URL in https://nbviewer.jupyter.org/ and share that URL here in the PR in a comment, when asking for review.
@QRemy

This comment has been minimized.

Copy link
Contributor Author

QRemy commented Nov 4, 2019

I splitted the datasets preparation from the analysis part. So now the analysis tutorial only read pre-made files. It takes about 15s to run.

Can we keep the preparation tutorials somewhere without listing it directly in the doc ?
I would like to give only a link at the end of this tutorial for users that would like to see how the files have been generated without having to go through 3 different tutorials.

@cdeil

This comment has been minimized.

Copy link
Member

cdeil commented Nov 6, 2019

@QRemy - You still have the commit with the output PNG in the notebook in this branch:

8fc7542#diff-cff32e9881017cdb9cc0e619eaa2b17dR197

Could you please squash the commits here to avoid adding this to our git history?

@cdeil

This comment has been minimized.

Copy link
Member

cdeil commented Nov 6, 2019

Looks like this is failing because you use an absolute path that only works on your machine:
https://travis-ci.org/gammapy/gammapy/jobs/607673321#L1835
You have to use a relative path to make this work on other machines.

This tutorial illustrates how to run a joint analysis with different datasets. We look at the gamma-ray emission from the Crab nebula between 10 GeV and 100 TeV. The spectral parameters are optimized by combining a 3D analysis of Fermi-LAT data, a ON/OFF spectral analysis of HESS data, and flux points from HAWC.
split datasets preparation from analysis


make dataset-index


remove prepare


change data path
@QRemy QRemy force-pushed the QRemy:add_joint_crab_notebook branch from 2b477e7 to 385c7d0 Nov 6, 2019
@cdeil cdeil moved this from To Do to In Progress in DOCUMENTATION Nov 6, 2019
@adonath adonath merged commit 60856ee into gammapy:master Nov 6, 2019
9 checks passed
9 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20191106.2 succeeded
Details
gammapy.gammapy (DevDocs) DevDocs succeeded
Details
gammapy.gammapy (Lint) Lint succeeded
Details
gammapy.gammapy (Test Python36) Test Python36 succeeded
Details
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
Details
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Details
DOCUMENTATION automation moved this from In Progress to Done Nov 6, 2019
@adonath

This comment has been minimized.

Copy link
Member

adonath commented Nov 6, 2019

Thanks @QRemy!

@cdeil

This comment has been minimized.

Copy link
Member

cdeil commented Nov 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
DOCUMENTATION
  
Done
4 participants
You can’t perform that action at this time.