Conversation
This reverts commit 8ccb49b.
There was a problem hiding this comment.
Pull request overview
This PR prepares SkyLLH for an upcoming IceCube public point-source “14-year” data release, while adjusting the public-data analysis pipeline to use improved background-energy PDF handling and updated energy-cut spline configuration.
Changes:
- Added a new
PublicData_14y_psdataset collection definition and a draft 14-year tutorial notebook. - Reworked parts of the publicdata PS analysis to derive energy-cut spline parameters from dataset aux data and modified background energy PDF evaluation via splines.
- Adjusted analysis parameter defaults (e.g.,
gamma_max) and added a few robustness tweaks (e.g., effective area boundary handling).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
skyllh/datasets/i3/link_dataset.sh |
Adds a helper script to symlink local data-release files. |
skyllh/datasets/i3/PublicData_14y_ps.py |
Introduces a new 14-year dataset collection definition (currently with hard-coded internal paths/docs). |
skyllh/datasets/i3/PublicData_10y_ps.py |
Adds aux-data parameters for energy-cut spline behavior; updates IC79 sin(dec) binning. |
skyllh/analyses/i3/publicdata_ps/utils.py |
Refactors FctSpline2D and replaces the energy-cut spline builder with a histogram/cumulative-threshold method. |
skyllh/analyses/i3/publicdata_ps/backgroundpdf.py |
Switches per-event background energy PDF evaluation to use a FctSpline2D spline. |
skyllh/analyses/i3/publicdata_ps/pdfratio.py |
Removes cap_ratio support from the public-data energy PDF ratio class. |
skyllh/analyses/i3/publicdata_ps/time_integrated_ps.py |
Drops cap_ratio/cut_sindec/spl_smooth plumbing; uses dataset aux-data driven energy-cut spline config. |
skyllh/analyses/i3/publicdata_ps/time_dependent_ps.py |
Similar parameter/default updates; uses dataset aux-data driven energy-cut spline config. |
skyllh/analyses/i3/publicdata_ps/signalpdf.py |
Adds a guard when reconstructed-energy PDFs are all zeros (with a warning). |
skyllh/analyses/i3/publicdata_ps/signal_generator.py |
Changes default behavior when cut_sindec is unset (now effectively all-sky). |
skyllh/analyses/i3/publicdata_ps/aeff.py |
Adds a lower-bound guard when determining the min-energy bin index. |
doc/sphinx/tutorials/publicdata_ps_14yr.ipynb |
Adds a draft 14-year tutorial notebook (currently “temp”/non-portable, with outputs). |
Comments suppressed due to low confidence (1)
skyllh/analyses/i3/publicdata_ps/signal_generator.py:388
- When
cut_sindecis not provided, the code now defaults to applying the energy cut over the full declination range ([-90, 90] deg). This contradicts the docstring (“energy cut in the IceCube southern sky”) and is a behavior change for signal injection. If the cut is meant to be southern-only by default, keep the default atcut_sindec = 0(or update docs/callers accordingly).
if cut_sindec is None:
logger.warn(
'No `cut_sindec` has been specified. The energy cut will be '
'applied in [-90, 90] deg.')
cut_sindec = np.sin(np.radians(90.1))
filter_mask = np.logical_and(
events['sin_dec'] < cut_sindec,
events['log_energy'] < spline(events['sin_dec']))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with TaskTimer(tl, 'Evaluating logE-sinDec histogram.'): | ||
| self._pd = self._hist_logE_sinDec[(logE_idx, sinDec_idx)] | ||
| # self._pd = self._hist_logE_sinDec[(logE_idx, sinDec_idx)] | ||
| self._pd = self._pdf_spline(tdm['log_energy'], tdm['sin_dec'], grid=False) |
There was a problem hiding this comment.
@chiarabellenghi not sure about this, it sounds correct but also shouldn't impact performance too much
| 'applied in [-90, 0] deg.') | ||
| cut_sindec = 0. | ||
| 'applied in [-90, 90] deg.') | ||
| cut_sindec = np.sin(np.radians(90.1)) |
There was a problem hiding this comment.
so this returns:
np.sin(np.radians(90.1))
np.float64(0.9999984769132877)
I guess we could just set it to 1.0 or 1.1?
There was a problem hiding this comment.
I think all this cut_sindec stuff can be removed. The new energy_cut_spline is doing the correct thing at all declinations
There was a problem hiding this comment.
Now that I think about it, for the new data release, the entire cut to inject events in the southern sky at meaningful energies is no longer needed. There, the effective area has been set to zero at low energies. However, we should keep this function so that the signal injection still works correctly for the 10yr data release.
Co-authored-by: Tomas Kontrimas <52071038+tomaskontrimas@users.noreply.github.com>
This PR integrates changes to the public data interface: