Skip to content

Conversation

@hiyoneda
Copy link
Contributor

@hiyoneda hiyoneda commented Oct 2, 2025

I started working on developing a class for the good time intervals (GTIs). GTI is used to determine the time intervals when data is analyzed. It will be incorporated into several classes related to the event selection and response matrix calculation.

@israelmcmc I added the first code for the GTI, which is put in the new directory ``good_time_intervals''. Do you think that it is reasonable? If it is better to move it to any existing directory, let me know.

@ckarwin, I think this is also related to the phase-resolved analysis, which you may already be working on. Once it is implemented in the cosipy, the phase-resolved analysis can be performed by determining the GTIs corresponding to the phase we want to analyze.

@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 92.30769% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.36%. Comparing base (e9dca56) to head (ab086c9).
⚠️ Report is 126 commits behind head on develop.

Files with missing lines Patch % Lines
cosipy/event_selection/good_time_interval.py 92.23% 8 Missing ⚠️
Files with missing lines Coverage Δ
cosipy/__init__.py 100.00% <ø> (ø)
cosipy/event_selection/__init__.py 100.00% <100.00%> (ø)
cosipy/event_selection/good_time_interval.py 92.23% <92.23%> (ø)

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@israelmcmc
Copy link
Collaborator

israelmcmc commented Oct 2, 2025

Thanks, @hirokiyoneda. It looks good so far. A couple of comments:

  • Maybe we should put it in a new "event_selection" folder. There will be many, we might as well group them all together.
  • It would be nice to have TimeRange, like sunpy's. I don't think it's enough to make sunpy a dependency, though.

@israelmcmc
Copy link
Collaborator

@ayshih are there plans to move TimeRange to astropy?

@ckarwin
Copy link
Contributor

ckarwin commented Oct 2, 2025

@hiyoneda I don't have any plans to work on the phase-resolved analysis myself, but I think other people are working on this.

I agree with the comment from @israelmcmc: with DC4 we plan to have numerous event selections, and so we should probably coordinate on how this should be organized.

Note that the dataIO class has a method to filter SAA events:

def cut_SAA_events(self, unbinned_data=None, output_name=None):

This relies on finding the good time intervals (or in this case, the bad time intervals):

def find_bad_intervals(self, times, values, bad_value=0.0):

@israelmcmc israelmcmc added event_selection Feature / Enhancement New functionality or improvement labels Oct 2, 2025
@hiyoneda
Copy link
Contributor Author

hiyoneda commented Oct 3, 2025

@israelmcmc OK, I renamed the directory to 'event_selection'. The TimeRange class sounds useful. I will consider using it if TimeRange is included in astropy in the future, but for now, it may be fine not to use it.

@ckarwin Yes, I know that the SAA filtering method was already implemented. I think that by using GTI as a separate file, the SAA filtering could become more flexible. In the case of NuSTAR, there are several options for the SAA filtering, which use different tracers of the SAA. Also, for COSI, we may want to compare our analysis with different SAA cuts. If we can filter events with a GTI file in a general way, we just need to prepare a new GTI file for a different SAA criterion. Then, the other part of the code does not need to be changed.

For this purpose, I added a new method "filter_good_data_with_gti" to the UnBinnedData class. I realize this implementation might change when the EventSelectionInterface is implemented in the future, but could you take a look and let me know if this approach seems reasonable for now?

@hiyoneda
Copy link
Contributor Author

hiyoneda commented Oct 3, 2025

@israelmcmc, I am wondering how the GTI should be included in the response matrix calculation. Currently, a response in the galactic coordinate for a point source is calculated using ScattMap. More specifically, the location of a source is first specified, and then spacecraftfile.scatt_map is called in many modules. Next, the calculated scattmap is used as one of the inputs of full_detector_response.get_point_source_response.

To include the GTI in this calculation, I think there are two possible solutions:

  1. We filter the spacecraft file with the given GTIs—that is, filtering the orientation information within the GTIs and generating a new spacecraft file. The pros of this solution are that only the spacecraft code needs to be modified. The cons are that we need to select a correct orientation file for each analysis, which may mean revising the concept that the orientation file does not need to be edited (if my understanding is correct).

  2. We add an optional parameter for the GTI as an input parameter of spacecraftfile.scatt_map. I think this approach is more understandable. The disadvantage is that it requires many (though trivial) changes in the classes that call spacecraftfile.scatt_map (polarization/polarization_asad, response/FullDetectorResponse, source_injector, COSILike) to add the GTI option in response-related methods.

I prefer the first option, but what do you think?

- removed is_in_gti
- renamed parameters for tstart and tstop
- add property methods for tstart and tstop
@israelmcmc
Copy link
Collaborator

@hiyoneda We talked about it, but just for the record, I agree we should go with option 1.

You load the information from the SC file, get a class instance, and then you can edit the live time of that instance. In the interfaces branched the SpacefractFile as renamed to SpacecraftHistory, as it doesn't necessarily need to read a file. The code was also refactored, besides changing the name, so this kind of operations should be easier.

@hiyoneda hiyoneda marked this pull request as ready for review October 22, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

event_selection Feature / Enhancement New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants