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 tutorial for pulsar analysis #2038

Merged
merged 4 commits into from Mar 7, 2019

Conversation

5 participants
@msjacob
Copy link

msjacob commented Feb 18, 2019

Here is a tutorial for pulsar analysis: phasogram, phase-resolved map and phase-resolved spectrum are done on a Vela observation simulation from the CTA DC1.

Marion Spir-Jacob added some commits Feb 18, 2019

@Bultako

This comment has been minimized.

Copy link
Member

Bultako commented Feb 19, 2019

Thanks @msjacob !
You can pretty-format the code cells with black in a consistent way, as it is done in the rest of the gammapy tutorials, using the following command gammapy jupyter --src=pulsar_analysis.ipynb black.

@Bultako Bultako added the docs label Feb 19, 2019

@registerrier
Copy link
Contributor

registerrier left a comment

Thank you @msjacob !
I have left a number of suggestions to shorten a bit and clarify the code. The main thing to do is to improve a bit on the explanations given.

"\n",
"# Defining the offset as the angular separation between the observation position and the target position\n",
"pos_obs = SkyCoord(table['GLON_PNT'], table['GLAT_PNT'], frame='galactic', unit='deg')\n",
"pos_target = SkyCoord.from_name('vela pulsar')\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

Avoid using SkyCorrd.from_name it does not work offline. Directly pass the coordinates.

"# Applying the mask\n",
"table = table[mask]\n",
"\n",
"id_obs_vela = table['OBS_ID'].data\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

To shorten a bit the tutorial and remove parts not directly connected to the pulsar timing analysis, I would suggest to simply load the only observation containing Vela in the CTA 1DC dataset shipped with gammapy and not do this observation selection.

"metadata": {},
"outputs": [],
"source": [
"# Making an angular cut to retain only the events within 0.2 deg\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

Maybe make a distinct cell and explain what is done as markdown.

"metadata": {},
"outputs": [],
"source": [
"# A phasogram is a histogram so you can set the binning the same way:\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

This comment would be better placed in the markdown cell above.

"\n",
"nbins = 30 # Number of bins\n",
"lim_phaso = (0, 1) # Range of phaseogram\n",
"size_bin = (lim_phaso[1] - lim_phaso[0])/nbins # Size of each bin\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

This cell isn't really needed. np.histogram returns the bin edges of your histogram. It seems simpler and safer to rely on these if you do not explicitly pass the bin edges.

nbins = 30
lim_phaso=(0,1)
values, bin_edges = np.histogram(phases, range = lim_phaso, bins=nbins)
midbins = 0.5*(bin_edges[:-1]+bin_edges[1:])
"# Loop to fill the ON- and OFF- maps\n",
"on_mask = np.where(np.logical_and(0.5 < phases, 0.6 > phases))[0]\n",
"fill_map_counts(on_map, one_obs_vela_select.select_row_subset(on_mask))\n",
"off_mask = np.where(np.logical_and(0.7 < phases, 1 > phases))[0]\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

A possibility to simplify a bit the code can be to use a 3D Map with a phase axis.

"fill_map_counts(off_map, one_obs_vela_select.select_row_subset(off_mask))\n",
"\n",
"# Create and fill excess map\n",
"excess_map = Map.create(binsz=0.1, map_type='wcs', skydir=pos_target, width=10.0)\n",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

You don't need to create a new Map but you should rather directly use Map arithmetics:

excess_map = on_map - alpha * off_map
"excess_map.data = on_map.data - 0.1/0.3 * off_map.data\n",
"\n",
"# Plot excess map\n",
"excess_map.plot()"

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

I would directly smooth it.

"cell_type": "markdown",
"metadata": {},
"source": [
"We can also do a phase-resolved spectrum."

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

Here you really need to explain what is going to happen both for analysis and code.

]
},
{
"cell_type": "code",

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 19, 2019

Contributor

Again explain a bit, what is done now. (You can also refer to the spectrum extraction tutorial but at least explain briefly what is done)

@adonath adonath added this to the 0.11 milestone Feb 19, 2019

@adonath adonath added this to To Do in Documentation via automation Feb 19, 2019

Marion Spir-Jacob
@msjacob

This comment has been minimized.

Copy link
Author

msjacob commented Feb 22, 2019

Hi and thanks @Bultako and @registerrier !
I changed the tutorial and did the black formatting.

@adonath adonath assigned adonath and registerrier and unassigned adonath Feb 22, 2019

@cdeil

This comment has been minimized.

Copy link
Member

cdeil commented Feb 26, 2019

@msjacob - Concerning the current version, see here:

I would suggest to remove the manual table of content at the top:

Table of contents:
I : Opening the data
II : Phasogram
III : Phase-resolved map
IV : Phase-resolved spectrum

and also the manual numbering in the headings.

It's hard to maintain manual numberings / tables of contents in our ~ dozen notebooks.

We have a rendering step which results in a navigation bar at the left:
https://docs.gammapy.org/0.10/notebooks/analysis_3d.html
We can improve that rendering step to have heading numbers later if we want.


In the introduction you mention PINT. Given that it's a very common term and there's another more popular python package with the same name: please add a link to the PINT package.

@adonath

This comment has been minimized.

Copy link
Member

adonath commented Mar 5, 2019

@msjacob Do you have time to address the remaining comments by @cdeil (if not I could take over...)? It would be great to get this PR merged for Gammapy v0.11. Thanks!

@msjacob

This comment has been minimized.

Copy link
Author

msjacob commented Mar 5, 2019

Hi @cdeil, @adonath ! It’s done, sorry for the delay!

@adonath
Copy link
Member

adonath left a comment

Thanks @msjacob! Do you have any further comments @registerrier?

@registerrier registerrier merged commit db9ef53 into gammapy:master Mar 7, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details

Documentation automation moved this from To Do to Done Mar 7, 2019

@registerrier

This comment has been minimized.

Copy link
Contributor

registerrier commented Mar 7, 2019

Thanks @msjacob !

@adonath adonath changed the title Tutorial for pulsar analysis Add tutorial for pulsar analysis Mar 26, 2019

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