Memory leak, downloading cells and SMEX GRB

[Yong2Sheng]

I updated the fast_ts_fit.py and the notebook for DC2 based the comments I received from the internal release.

Update 1

Following the suggestion by @israelmcmc, the GRB in the notebook has been replaced by the SMEX one simulated by @eneights.

Update 2

The memory usage is improved.

• The memory peak by fitting GRB is less than 15GB using 40 cores on Clemson Palmetto Cluster
• The memory peak by fitting Crab is less than 8GB using 40 cores on Clemson Palmetto Cluster

If you use less cores, the memory usage will be less at the cost of longer computational time.

Update 3

I added the downloading cells in the notebook so that the files needed will be downloaded to the same directory of the notebook. The size of the files are also mentioned in the cells.

The zipped response will be unzipped and deleted by the cells.

Note

You will see some commented lines in fast_ts_fit.py that are used for speed benchmark. They might be used in the future so I want to keep them for now.

[ckarwin]

Hi @Yong2Sheng, I started reviewing your PR, and so far things look good. A few comments below.

1)

special note: although there is "1s" in the name of the background file, it's actually a 3-month backgroud.

What is the point of this? It is better to just name things what they are.

2)

# clear redundant data from RAM to prevent memory leak
del bkg_full
del ori_full
_ = gc.collect()

I don't know if I would really consider this a "memory leak". You're just removing un-needed files from RAM. You can probably remove "to prevent memory leak" from your comment. Also, I think you can just run gc.collect() without assigning it to a dummy variable.

3) I have some questions regarding "Point_source_resonse.ipynb". I think it will probably discuss this in person first at the cosipy meeting.

[Yong2Sheng]

Hi @ckarwin,

Thank you for your comments and suggestions!

Comment 1

What is the point of this? It is better to just name things what they are.

I updated this file on Wasabi and the cell. The comment explaining the "1s" in the file name has been removed from the downloading cell.

Comment 2

1. You can probably remove "to prevent memory leak" from your comment.

This phrase has been removed from the notebook

2. I think you can just run gc.collect() without assigning it to a dummy variable.

If I don't assign a dummy variable, the gc.collect() will return a number to the user, which might cause confusion. This number isn't important here, so I use the dummy variable to prevent the output. The returned is the number of unreachable objects found.

Other changes

1. I put the response related tutorials files under docs/tutorials/response.
2. The name of Point_source_resonse.ipynb is corrected to Point_source_response.ipynb.
3. I put GRB_source_injector.ipynb under docs/tutorials/source_injector.
4. The index.rst file is updated.

[ckarwin]

Thanks, @Yong2Sheng! This looks really good. A couple more requests:

1) Can you go ahead and delete Intro.ipynb?

2) Can you point me to where you are using CygX1_source_path_in_SC_frame.npy and CygX1_DwellMap.fits? I still don't see it.

[Yong2Sheng]

Hi @ckarwin,

Comment 1

Can you go ahead and delete Intro.ipynb?

It's deleted now.

Comment 2

Can you point me to where you are using CygX1_source_path_in_SC_frame.npy and CygX1_DwellMap.fits? I still don't see it.

Sorry, I now realized that they are intermediate files generated by Point_source_response.ipynb. They are deleted now.

Thank you very much!

[ckarwin]

I actually found that one of the files is used in Point_source_response.ipynb, although the cell is commented out:

#The idea is the same as dwell time map: if you already have the dwell time map, you don't need to run all the steps above, 
#simply load the dwell timemap here by speficifying the dwell_map parameter.

# _ =  ori.get_psr_rsp(response =response_file_dir,
#                      dwell_map = "CygX1_DwellMap.fits")

# errors to be fixed!

Do you still want this cell? If so, you might still need the file. Or is this something that needs to be fixed later?

[Yong2Sheng]

Hi @ckarwin,

I tested the cell, there is no error when loading the dwell time map now, so the error has been fixed. But I still want to keep this cell since it tells the users that they can load the dwell time map instead calculating it.

Here are the main changes:

1. I updated the path to the inputs needed for the point source response notebook.
2. I kept the cell, but I removed the line of # errors to be fixed!
3. I upload the dwell time map file CygX1_DwellMap.fits to docs/tutorials/response

Thank you very much!

[ckarwin]

Hi @Yong2Sheng,

Ok, thanks for updating that. I have another question now. What is the difference between DetectorResponse.ipynb and Point_source_response.ipynb? It seems that the point source response is already handled in the former notebook, and so why do we need the latter notebook? Does that latter notebook provide any additional information that is not already covered in the first notebook, or is it just another example? I think it's fine if it's just another example, but it would be good to comment on this at the beginning of Point_source_response.ipynb, i.e. make it clear how this notebook relates to the other one.

Sorry for all the questions, but I think it's good to handle these issues now, while we are already working on related things. Otherwise, it will probably be a while before we revisit these small details.

[Yong2Sheng]

Hi @ckarwin,

This is a good question. @israelmcmc initially made DetectorResponse.ipynb, which includes the part that deals with the point source response.

I think the differences are:

1. The point source response was hard coded in DetectorResponse.ipynb while Point_source_response.ipynb uses the module SpacecraftFile.py to present how to generate the point source response.

2. DetectorResponse.ipynb has additional information on:

   • Explanation of the detector response format and meaning
   • Visualizing the response
3. Point_source_response.ipynb has additional information on:
   • Reading and visualizing the orientation file
   • Converting the coordinates from galactic frame to the spacecraft frame
   • Exporting the effective area and energy redistribution matrix to the format that can be read by XSPEC.

So I would conclude that SpacecraftFile.py and Point_source_response.ipynb come fromDetectorResponse.ipynb but:

• DetectorResponse.ipynb explains the response from an underlying view.

• Point_source_response.ipynb extends the usage of the point source response.

Maybe we should keep both of them and provide information for the users to distinguish them? What do you think? @ckarwin @israelmcmc

Thank you very much!

[israelmcmc]

Good point. I think we should reorganize these 2 tutorials. When I wrote the DetectorResponse.ipynb we didn't yet have the SpacecrateFile, but I still wanted to show how it would be convolved in order to obtain the expected excess.

This is what I have in mind (from the tutorial docs page), which I think it's a logical way to present it for someone starting now:

https://cositools.github.io/cosipy/tutorials/index.html

2. Spacecraft orientation and location (Point_source_resonse.ipynb)

   • SC file format and manipulation it —e.g. get a time range, rebin it.
   • The dwell time map and how to obtain it
   • The scatt map and how to obtain it

3. Detector response and signal expectation (DetectorResponse.ipynb)

   • Explanation of the detector response format and meaning
   • Visualizing the response
   • Convolving the detector response with a point source model (location + spectrum) + spacecraft file to obtain the expected signal counts. Both in SC and galactic coordinates.

What do you think? I can update DetectorResponse.ipynb in order to obtain the response with a Spacecraft File. @Yong2Sheng can you update Point_source_resonse.ipynb (which I think needs to change its name to SpacecraftFile.ipynb) so it explains the points above?

[Yong2Sheng]

Hi @israelmcmc,

Thank you for the suggestions! For the Point_source_resonse.ipynb, I should:

• Change the file name
• Mention the differences between SpacecraftFile.ipynb and DetectorResponse.ipynb

Is it correct?

[israelmcmc]

Sorry for the confusion, @Yong2Sheng. I meant to modify Point_source_resonse.ipynb so it describes:

• SC file format and manipulation it —e.g. get a time range, rebin it.
• The dwell time map and how to obtain it
• The scatt map and how to obtain it

I think the first 2 bullet points are already there, it would mean adding the scatt map and removing the PSR calculation. I'll PM on slack to coordinate about this.

[Yong2Sheng]

@israelmcmc Sounds good!

[israelmcmc]

To summarize our chat:

• What is currently called "Point_source_resonse.ipynb" will change names to "SpacecraftFile.ipynb" and focus on the coordinates, orientation, etc. (described in 2. above)
• I'll add the explanation of the scatt map to SpacecraftFile.ipynb
• @Yong2Sheng will move the actual point source calculation to “DetectorResponse.ipynb” (that is, replacing my code with a fake orientation history)
• Will do this on new PRs. This PR can be (and should be) merged first.

[ckarwin]

Sounds good. Thanks for taking care of this, @Yong2Sheng @israelmcmc!

One last request for this PR @Yong2Sheng, can you remove this folder: https://github.com/Yong2Sheng/cosipy_SpacecraftFile/tree/fast_ts_map_galactic/docs/tutorials/data?

I checked with @eneights and it's not being used.

[Yong2Sheng]

Hi @ckarwin,

I deleted the folder docs/tutorials/data.

Although it says that there is a change in DetectorResponse.ipynb, I didn't do anything to it. What changed is the metadata of the notebook (python kernel name, python version of the kernel I used).