cositools / cosipy

The COSI high-level data analysis tools
Apache License 2.0
3 stars 16 forks source link

Add cells to download files in grb & crab spectral fit notebooks #144

Closed eneights closed 4 months ago

eneights commented 4 months ago

Add cells in the grb & crab spectral fitting notebooks to download the necessary files from wasabi

ckarwin commented 4 months ago

Hi @eneights, Thanks for making these changes. I reviewed your PR, and all the new cells are working. There are still some small details that I think need to be updated though, specified below. Also, it looks like the line fit example has not been updated yet. Were you planning to do this as well?

Both Crab and GRB notebooks:

1) > To run this, you need the following files, which can be found on wasabi and downloaded using the first few cells of this notebook:

Most users won't have access to wasabi, and so I think you can just say that the files need to be downloaded using the first few cells of the notebook.

2) You still have your personal data path in the first cell of the crab notebook. I think the default can just be ".", which is the working directory. Or "path/to/files", as you have in the GRB notebook.

3) Can you please also add the file sizes for each cell, in order to give users a sense of how long the download will take? They can be found on wasabi.

4) I think it would be sufficient to add a comment before the cells specifying that the files should only be downloaded if not previously downloaded, instead of repeating this for each cell.

eneights commented 4 months ago

Thanks @ckarwin! I made those changes.

The point source line fitting notebook should probably be deleted, do you want me to do that? It's for the Compton sphere and the extended source 511 notebook replaces it.

ckarwin commented 4 months ago

Thanks for making the changes, @eneights.

Ok, I see. I think it's probably ok to remove the line fitting notebook. Let's double check with @israelmcmc, do you see any problems with this?

israelmcmc commented 4 months ago

I also agree it's good to remove it. Conceptually, the jump from a PL or band function to a Dirac delta (or a narrow Gaussian) is not that big, so I think the users will not get lost if the line emission analysis is introduced together with the extended source emission ---which is the realistic use case anyways. The 511-emitting "GRB" had its purpose when were trying to figure out the implementation and it allowed us to tackle one problem at a time, but at this point, it's better to reduce the number of notebooks that we need to maintain.

ckarwin commented 4 months ago

Ok, @eneights, please go ahead and remove the line fit NB, and then I can merge this PR.

In the second line of the extended source NB it says:

For a general introduction into spectral fitting with cosipy, see the continuum_fit and line_fit tutorials.

Can you also delete:

and line_fit tutorials

eneights commented 4 months ago

@ckarwin I deleted the line fit tutorial and its references in the extended source notebook and other_examples.rst.