fermiPy / fermipy

Fermi-LAT Python Analysis Framework
http://fermipy.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
51 stars 53 forks source link

Fit to localization contour failed #548

Closed jballet closed 4 months ago

jballet commented 1 year ago

I am trying to routinely localize all catalog sources in GAL coordinates. In about 10% of my calls I get the warning message 2023-08-03 18:44:30 WARNING GTAnalysis._localize(): Fit to localization contour failed. 2023-08-03 18:44:30 WARNING GTAnalysis._localize(): Localization failed. Keeping existing position. and the fit_success entry turns to False, so my run stops. It appears that fermipy did find a best localization (it even provides it in the log) but, as the warning says, failed to extract an error ellipse from the TS contours (and indeed, the error plotted on the localization plot looks wrong). This is the first (large-scale) localization map (called _localize). 03bp-0207_localize This is the close-up (called _localize_peak) that I think illustrates the problem. 03bp-0207_localize_peak I suspect the algorithm to fit the ellipse is not robust enough. This warning is sometimes (not always) accompanied by fermi_py_patch514/lib/python3.9/site-packages/scipy/optimize/ _lsq/common.py:147: RuntimeWarning: invalid value encountered in double_scalars alpha = max(0.001 alpha_upper, (alpha_lower alpha_upper)**0.5) I should add that I am leaving a lot of freedom in the neighboring sources at the localization step (because it is in theory better to do so). But it does not seem to be the problem because the fit itself does not fail and the TS map looks smooth. The failure is very sensitive to details. If I fix a neighboring source it will often go away. Consequently I have not been able to streamline the input as much as I normally do and the input srcmap file is quite big (although the computing time is only 3 minutes or so). The test harness can be found at this link until August 18. Call fermipy_1073.py. The big files (gll_iem_v07.fits and ft2_12years_reduced.fits) are not included. The former is the regular diffuse model. The latter is available at SLAC at /nfs/farm/g/glast/g/catalog/P8_P305. The products of my run (PNG files and log) are included in the tgz file. The fermipy version I use is the patch514 that was created to fix issue 514. It is called 1.2.0+2.gfa69 by conda list. It is associated with FermiTools 2.2.0.

jballet commented 1 year ago

I have managed to reproduce the failure on a much smaller file, by selecting only 1 to 10 GeV and a 200x200 piece of the source map (extracted from the previous source map, so the projection center is not at the center of the map). The test harness is available at this link. In that case all other sources and the isotropic are completely fixed and included into the FixedSources component. You will see by comparing with the PNG files in the previous example that the source is not found exactly at the same place. The circle is larger and much closer to what it should be (when it works). The failure is still sensitive to the exact pixellization, because when I recreate the map from scratch (with projection center at the center of the map) it does not fail any more (and the best fit position is again at a different place, although all the positions are within the 68% error circle).

jballet commented 1 year ago

This morning I tried reverting to the original fermipy 1.2.0. Many sources passed localization then (including that above). However my jobs then failed on another source in the same RoI, with the "Best-fit position outside of search region" message, and the same weird localization maps as in issue 514. So it would seem that this new issue is somehow related to the fix to issue 514. A fraction of sources did fail with the same "Fit to localization contour failed" error message as above in fermipy 1.2.0. But those few I looked into appeared like a consequence of the weird localization maps.

jballet commented 1 year ago

Because of the original suspicion (for issue 514) that the problem was specific to GAL coordinates, I have launched a full run (with many RoIs) in CEL coordinates. A similar fraction of RoIs failed with the same errors (under fermipy 1.2.0 or patch514). They do not necessarily fail on the same sources in GAL and CEL, as expected from the sensitivity to pixellization described above. But there is no indication that the localization works any better in CEL coordinates.

jballet commented 6 months ago

The test harness is again available at this link until April 6. The same link also contains the test harness to issue 514.

ndilalla commented 5 months ago

Hi @jballet, I just pushed a new fix to branch fix_loc_issue_514 trying to improve the robustness of the localization fit which was causing this issue and (part of) #514. With this fix I was able to successfully run both the test cases provided. Could you please try to run your tests and see if the issues are still there? You can install this development version by running from inside the fermipy environment:

pip install -U git+https://github.com/fermiPy/fermipy@fix_loc_issue_514
jballet commented 5 months ago

Hi Niccolo, When running it on the small test harness that failed the original patch514, I get the warning .../lib/python3.9/site-packages/scipy/optimize/_lsq/common.py:147: RuntimeWarning: invalid value encountered in scalar power alpha = max(0.001 alpha_upper, (alpha_lower alpha_upper)**0.5) Do you see that as well? The localization proceeds successfully anyway. I will now try on more sources.

jballet commented 4 months ago

The new version failed on a much smaller fraction of sources (about 0.4%). Nearly all were faint sources (TS < 25), in which the TS map did not look nice, and the detailed TS map with free local sources (shown in the localize_peak plot) was inconsistent with the simple TS map (shown in the localize plot). This is probably unavoidable in confused regions, and I can handle it by ignoring localization for those faint sources. Two failed because the best position was too far from the starting point. This was obvious on the TS maps, and could be handled manually. I am happy with this new version.

ndilalla commented 4 months ago

Thanks @jballet for running your test! That's great to hear. I'll go ahead and open the pull request to merge this fix to the master branch.