Skip to content

Conversation

@avalluvan
Copy link
Contributor

Preview
DetectorResponse class has been expanded with new functionalities
FullDetectorResponse class has been cleaned up, comments have been added, and interpolation of NuLambda axis has been made possible.

DetectorResponse._set_mapping

  • Sets the mapping between target value (physical units) and axes labels (any binning scheme that is employed) to facilitate interpolated response calculation
  • Generalized implementation to allow testing of various response file binning schemes

_get_all_interp_weights

  • Get weights for interpolation along multiple axes for target value provided as a dictionary target
  • Returns in the same format as HealpixAxis and Axis. (pixels, weights)

transform_eps_to_Em and transform_Em_to_eps

  • Placeholder function to transform between eps coordinates (new binning scheme) and Em coordinates (physical units)
  • Generalized implementation to allow expanding to other transformations
  • Can also be implemented for other axes

get_interp_response

  • Calculates interpolated response value for a target value provided as a dictionary target
  • Example: target = {'Ei': 511*u.keV, 'Em': 600*u.keV, 'Phi': 12*u.deg, 'PsiChi': 385}
  • Can calculate vector along 'Ei' axis if only three target values are given.
  • Currently does not support vector returns along other axes. As the other axes are part of the measured data space, do we really expect to require that?

FullDetectorResponse.getitem

  • Restructured code to do away with repeated logic

self.unbinned

  • Just a placeholder variable for now. It can be used to impose restrictions on which features are accessible for binned vs unbinned analysis

get_point_source

  • Added interpolation code along NuLambda axis that was on the TODO list
  • deepcopy statement should be removable but I do not know how to do it. The coordsys fields are in conflict dr.axes['PsiChi'].coordsys == response[p].axes['PsiChi'].coordsys returns False.

Suggested changes for histpy

  • Interpolations in log-space should not follow x-x1, x2-x convention. We should switch to x/x1, x2/x convention.
  • FullDetectorResponse AXES/TYPE attribute is inconsistent
    • For healpix, it is “healpix”. For axis, it is “linear” or “log”.

Questions about next steps

  • Do we expect to reparametrize the model space?
  • Should reparametrization handling codes be included in the same DetectorResponse class or should we branch out to a new file? Right now, DetectorResponse is fully back-compatible
  • How do you vectorize eps_to_Em conversion when the function parameters change for different values of Ei
  • Should we try out preferential weighting along diagonal direction (and reduce the weights along anti-diagonal direction)
  • Do we want to use sparse matrices for scatt_map?
  • How do we remove the deepcopy statement in FullDetectorResponse.get_point_source()
  • When will we deprecate exposure_map in favor of coords + scatt map for both local and inertial coords?

@avalluvan avalluvan force-pushed the feature/general_response branch from c957980 to 5f4e3a9 Compare April 8, 2025 06:11
@avalluvan avalluvan force-pushed the feature/general_response branch from 5f4e3a9 to e88c974 Compare April 8, 2025 06:23
@israelmcmc israelmcmc added pull-request-needs-reviewer No reviewer assigned Feature / Enhancement New functionality or improvement labels Sep 11, 2025
@israelmcmc israelmcmc self-assigned this Sep 15, 2025
@israelmcmc
Copy link
Collaborator

@avalluvan I'll come back to this PR after #369. I feel that a lot of the goals of this PR can benefit from the interfaces refactoring.

@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 Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature / Enhancement New functionality or improvement pull-request-waiting-for-reviewer The ball is on the reviewer side.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants