Skip to content

Add ARMSelector event selection class#593

Open
Mustafa-hub-maker wants to merge 4 commits into
cositools:developfrom
Mustafa-hub-maker:add-arm-selector
Open

Add ARMSelector event selection class#593
Mustafa-hub-maker wants to merge 4 commits into
cositools:developfrom
Mustafa-hub-maker:add-arm-selector

Conversation

@Mustafa-hub-maker

@Mustafa-hub-maker Mustafa-hub-maker commented Apr 28, 2026

Copy link
Copy Markdown

This PR adds a first ARMSelector implementation for COSIpy event selection.

The selector applies an ARM cut using:

ARM = angular separation(source direction, event axis) - Phi

Updated implementation uses:

  • Chi galactic
  • Psi galactic
  • Phi
  • all stored in radians

Validated locally on GRB_bn110605183:

  • ARM cut: ±13 deg
  • GRB total events: 2,776
  • GRB selected events: 554
  • GRB selected fraction: 0.1996
  • ARM distribution is centered close to 0°, as expected

Tests were updated accordingly:

  • ARMSelector tests: 5/5 passed locally
  • Related event-selection tests: 10/10 passed locally

Note:
This is the validated FITS-column implementation. A remaining refinement is adapting it to the full ComptonDataSpaceInSCFrameEventInterface, where SpacecraftHistory is used internally for the sky-to-spacecraft transformation.

@codecov

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.02%. Comparing base (a34afcd) to head (296d8e6).
⚠️ Report is 191 commits behind head on develop.

Files with missing lines Coverage Δ
cosipy/event_selection/__init__.py 100.00% <100.00%> (ø)
cosipy/event_selection/arm_selection.py 100.00% <100.00%> (ø)

... and 47 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
Copy Markdown
Collaborator

Thanks for working on this @Mustafa-hub-maker.

It's on the right path, but the ARMSelector is not yet working as an EvenSelectionInterface implementation. For that to work, _select needs to accept an EvenDataInterface object, and use it for the calculation. Right now, it's taking a "table-like" object, which doesn't conform with the EvenDataInterface protocol definition. Also, the __init__ method shouldn't take the chi/psi/phi-col parameters, those are also obtained through the input EvenDataInterface object in _select. You can use existing TimeSelector class as a model for what to do.

On a separate note, it would be good if you use the existing RelativeCDSCoordinates class to compute the ARM. If you feel that your code improves upon what's already available, feel free to modify RelativeCDSCoordinates. I just want to avoid duplicated code if possible.

@israelmcmc israelmcmc self-requested a review June 1, 2026 20:37
@israelmcmc israelmcmc added pull-request-waiting-for-reviewer The ball is on the reviewer side. and removed pull-request-needs-reviewer No reviewer assigned labels Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants