Integration in the fitting procedure#87
Conversation
…ort for positionnal arguments in fitting class.
MSCarrasco
left a comment
There was a problem hiding this comment.
Hi @gabemery, just one comment on my side about the offset axis computation
| exp_ds = exp_map_background.downsample(self.oversample_map, preserve_counts=True) | ||
| exp_total_ds = exp_map_background_total.downsample(self.oversample_map, preserve_counts=True) | ||
|
|
||
| # 3) build final offset axes (matching Grid3DAcceptanceMapCreator) |
There was a problem hiding this comment.
I feel like this should be helper function in the base_acceptance_map_creator, if it is used in multiple cases
There was a problem hiding this comment.
It could be added in Grid3DAcceptanceMapCreator since it assumes 2 spatial dimension and the Fit class inherits the Grid3D. I will check and implement your proposal
|
|
||
| bin_width_x = np.repeat(extended_offset_axis_x.bin_width[:, np.newaxis], extended_offset_axis_x.nbin, axis=1) | ||
| bin_width_y = np.repeat(extended_offset_axis_y.bin_width[np.newaxis, :], extended_offset_axis_y.nbin, axis=0) | ||
| solid_angle = 4.0 * (np.sin(bin_width_x / 2) * np.sin(bin_width_y / 2)) * u.steradian |
There was a problem hiding this comment.
Do we want to take into account projection effect ?
I would said not mandatory for this PR, but I would leave a comments highlighting this point.
There was a problem hiding this comment.
I think it makes sense, but this is out of the scope of this PR. Since it also concerns existing functionnality (namely the background normalisation from counts).
Instead of a comment lost in the code I think an issue is better.
MSCarrasco
left a comment
There was a problem hiding this comment.
All good to me, thanks!
Implement a simple integration in the fitting procedure. Allow to fit the actual physical distribution of events instead of the convolution with bin size. Improve stability with varying, or non-uniform, binning.
Also includes a small fix and small improvements.
Closes #77