nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: gammapy/gammapy
Pull request #5301 (2024-06-11 15:51:08)

Backport PR #5300 on branch v1.2.x (Fix wrap in PiecewiseNormSpatialModel)
Backport PR #5300: Fix wrap in PiecewiseNormSpatialModel


Patch:
diff --git a/gammapy/modeling/models/spatial.py b/gammapy/modeling/models/spatial.py
index d190aceb1..aa0cae5c9 100644
--- a/gammapy/modeling/models/spatial.py
+++ b/gammapy/modeling/models/spatial.py
@@ -1678,9 +1678,9 @@ class PiecewiseNormSpatialModel(SpatialModel):
 
     def __init__(self, coords, norms=None, interp="lin", **kwargs):
         self._coords = coords.copy()
-        self._coords["lon"] = Angle(coords.lon).wrap_at(0 * u.deg)
-        self._wrap_angle = (coords.lon.max() - coords.lon.min()) / 2
-        self._coords["lon"] = Angle(coords.lon).wrap_at(self._wrap_angle)
+        lon = Angle(coords.lon).wrap_at(0 * u.deg)
+        self._wrap_angle = (lon.max() - lon.min()) / 2
+        self._coords["lon"] = Angle(lon).wrap_at(self._wrap_angle)
         self._interp = interp
 
         if norms is None:
@@ -1726,6 +1726,7 @@ class PiecewiseNormSpatialModel(SpatialModel):
         coords = [value.value for value in self.coords._data.values()]
         # TODO: apply axes scaling in this loop
         coords = list(zip(*coords))
+        lon = Angle(lon).wrap_at(0 * u.deg)
         lon = Angle(lon).wrap_at(self._wrap_angle)
         # by default rely on CloughTocher2DInterpolator
         # (Piecewise cubic, C1 smooth, curvature-minimizing interpolant)


Test patch:
diff --git a/gammapy/modeling/models/tests/test_spatial.py b/gammapy/modeling/models/tests/test_spatial.py
index 4797e39ea..0fe757c78 100644
--- a/gammapy/modeling/models/tests/test_spatial.py
+++ b/gammapy/modeling/models/tests/test_spatial.py
@@ -706,6 +706,7 @@ def test_piecewise_spatial_model_gc():
 
     model_dict = model.to_dict()
     new_model = PiecewiseNormSpatialModel.from_dict(model_dict)
+    assert model_dict == new_model.to_dict()
 
     assert_allclose(new_model.evaluate_geom(geom.to_image()), expected, atol=1e-5)
 
@@ -720,11 +721,14 @@ def test_piecewise_spatial_model():
         geom = WcsGeom.create(
             skydir=(lon, 2.3), npix=(2, 2), binsz=0.3, frame="galactic"
         )
+
         coords = MapCoord.create(geom.footprint)
+        print(coords["lon"])
         coords["lon"] *= u.deg
         coords["lat"] *= u.deg
 
         model = PiecewiseNormSpatialModel(coords, frame="galactic")
+        print(model.coords["lon"].deg)
 
         assert_allclose(model(*geom.to_image().center_coord), 1.0)
 
@@ -741,6 +745,7 @@ def test_piecewise_spatial_model():
 
         model_dict = model.to_dict()
         new_model = PiecewiseNormSpatialModel.from_dict(model_dict)
+        assert model_dict == new_model.to_dict()
 
         assert_allclose(new_model.evaluate_geom(geom.to_image()), expected, atol=1e-5)
Repository: gammapy/gammapy
Pull request #5323 (2024-06-18 14:23:41)

Fix covariance for CompoundSpectralModel 
Set correctly the sub-covariance on the individual models of   CompoundSpectralModel.
As suggested by @adonath this should go in before  #5316, then the Mixin class can be introduced to reduce code duplication. 


Patch:
diff --git a/gammapy/modeling/models/spectral.py b/gammapy/modeling/models/spectral.py
index fa5552436..222db3025 100644
--- a/gammapy/modeling/models/spectral.py
+++ b/gammapy/modeling/models/spectral.py
@@ -27,6 +27,7 @@ from gammapy.utils.interpolation import (
 )
 from gammapy.utils.roots import find_roots
 from gammapy.utils.scripts import make_path
+from ..covariance import Covariance
 from .core import ModelBase
 
 log = logging.getLogger(__name__)
@@ -778,6 +779,31 @@ class CompoundSpectralModel(SpectralModel):
         op = getattr(operator, data["operator"])
         return cls(model1, model2, op)
 
+    def _check_covariance(self):
+        if not self.parameters == self._covariance.parameters:
+            self._covariance = Covariance.from_stack(
+                [model.covariance for model in self._models]
+            )
+
+    @property
+    def covariance(self):
+        """Covariance as a `~gammapy.modeling.Covariance` object."""
+        self._check_covariance()
+
+        for model in self._models:
+            self._covariance.set_subcovariance(model.covariance)
+
+        return self._covariance
+
+    @covariance.setter
+    def covariance(self, covariance):
+        self._check_covariance()
+        self._covariance.data = covariance
+
+        for model in self._models:
+            subcovar = self._covariance.get_subcovariance(model.covariance.parameters)
+            model.covariance = subcovar
+
 
 class PowerLawSpectralModel(SpectralModel):
     r"""Spectral power-law model.


Test patch:
diff --git a/gammapy/modeling/models/tests/test_spectral.py b/gammapy/modeling/models/tests/test_spectral.py
index d8f3e21a0..650a2268d 100644
--- a/gammapy/modeling/models/tests/test_spectral.py
+++ b/gammapy/modeling/models/tests/test_spectral.py
@@ -774,6 +774,20 @@ def test_template_spectral_model_compound():
     assert np.allclose(new_model(energy), 2 * values)
 
 
+def test_covariance_spectral_model_compound():
+    model = TEST_MODELS[2]["model"] + TEST_MODELS[1]["model"]
+    covar = np.eye(len(model.parameters))
+    covar[0, -1] = 1.0
+    covar[-1, 0] = 1.0
+    covar[0, 1] = 1.0
+    covar[-1, -3] = 1.0
+
+    model.covariance = covar
+    assert_allclose(model.covariance.data, covar)
+    assert_allclose(model.model1.covariance.data, covar[:3, :3])
+    assert_allclose(model.model2.covariance.data, covar[-3:, -3:])
+
+
 def test_evaluate_spectral_model_compound():
     energy = [1.00e06, 1.25e06, 1.58e06, 1.99e06] * u.MeV
     model = TEST_MODELS[-2]["model"]
Repository: gammapy/gammapy
Pull request #5330 (2024-06-19 10:58:57)

Backport PR #5323 on branch v1.2.x (Fix covariance for CompoundSpectralModel )
Backport PR #5323: Fix covariance for CompoundSpectralModel 


Patch:
diff --git a/gammapy/modeling/models/spectral.py b/gammapy/modeling/models/spectral.py
index 146b10c7a..772bad74f 100644
--- a/gammapy/modeling/models/spectral.py
+++ b/gammapy/modeling/models/spectral.py
@@ -26,6 +26,7 @@ from gammapy.utils.interpolation import (
 )
 from gammapy.utils.roots import find_roots
 from gammapy.utils.scripts import make_path
+from ..covariance import Covariance
 from .core import ModelBase
 
 log = logging.getLogger(__name__)
@@ -759,6 +760,31 @@ class CompoundSpectralModel(SpectralModel):
         op = getattr(operator, data["operator"])
         return cls(model1, model2, op)
 
+    def _check_covariance(self):
+        if not self.parameters == self._covariance.parameters:
+            self._covariance = Covariance.from_stack(
+                [model.covariance for model in self._models]
+            )
+
+    @property
+    def covariance(self):
+        """Covariance as a `~gammapy.modeling.Covariance` object."""
+        self._check_covariance()
+
+        for model in self._models:
+            self._covariance.set_subcovariance(model.covariance)
+
+        return self._covariance
+
+    @covariance.setter
+    def covariance(self, covariance):
+        self._check_covariance()
+        self._covariance.data = covariance
+
+        for model in self._models:
+            subcovar = self._covariance.get_subcovariance(model.covariance.parameters)
+            model.covariance = subcovar
+
 
 class PowerLawSpectralModel(SpectralModel):
     r"""Spectral power-law model.


Test patch:
diff --git a/gammapy/modeling/models/tests/test_spectral.py b/gammapy/modeling/models/tests/test_spectral.py
index 39ca0c727..95b84c4c5 100644
--- a/gammapy/modeling/models/tests/test_spectral.py
+++ b/gammapy/modeling/models/tests/test_spectral.py
@@ -755,6 +755,20 @@ def test_template_spectral_model_compound():
     assert np.allclose(new_model(energy), 2 * values)
 
 
+def test_covariance_spectral_model_compound():
+    model = TEST_MODELS[2]["model"] + TEST_MODELS[1]["model"]
+    covar = np.eye(len(model.parameters))
+    covar[0, -1] = 1.0
+    covar[-1, 0] = 1.0
+    covar[0, 1] = 1.0
+    covar[-1, -3] = 1.0
+
+    model.covariance = covar
+    assert_allclose(model.covariance.data, covar)
+    assert_allclose(model.model1.covariance.data, covar[:3, :3])
+    assert_allclose(model.model2.covariance.data, covar[-3:, -3:])
+
+
 def test_evaluate_spectral_model_compound():
     energy = [1.00e06, 1.25e06, 1.58e06, 1.99e06] * u.MeV
     model = TEST_MODELS[-2]["model"]
Repository: gammapy/gammapy
Pull request #5338 (2024-06-20 17:26:48)

Backport PR #5323 on branch v1.0.x (Fix covariance for CompoundSpectralModel )
Backport PR #5323 on branch v1.0.x (Fix covariance for CompoundSpectralModel )




Patch:
diff --git a/gammapy/modeling/models/spectral.py b/gammapy/modeling/models/spectral.py
index 320dd6b68..1d3c196c5 100644
--- a/gammapy/modeling/models/spectral.py
+++ b/gammapy/modeling/models/spectral.py
@@ -22,6 +22,7 @@ from gammapy.utils.interpolation import (
 )
 from gammapy.utils.roots import find_roots
 from gammapy.utils.scripts import make_path
+from ..covariance import Covariance
 from .core import ModelBase
 
 log = logging.getLogger(__name__)
@@ -683,6 +684,31 @@ class CompoundSpectralModel(SpectralModel):
         op = getattr(operator, data["operator"])
         return cls(model1, model2, op)
 
+    def _check_covariance(self):
+        if not self.parameters == self._covariance.parameters:
+            self._covariance = Covariance.from_stack(
+                [model.covariance for model in self._models]
+            )
+
+    @property
+    def covariance(self):
+        """Covariance as a `~gammapy.modeling.Covariance` object."""
+        self._check_covariance()
+
+        for model in self._models:
+            self._covariance.set_subcovariance(model.covariance)
+
+        return self._covariance
+
+    @covariance.setter
+    def covariance(self, covariance):
+        self._check_covariance()
+        self._covariance.data = covariance
+
+        for model in self._models:
+            subcovar = self._covariance.get_subcovariance(model.covariance.parameters)
+            model.covariance = subcovar
+
 
 class PowerLawSpectralModel(SpectralModel):
     r"""Spectral power-law model.


Test patch:
diff --git a/gammapy/modeling/models/tests/test_spectral.py b/gammapy/modeling/models/tests/test_spectral.py
index 9e7c79493..b0ea84f56 100644
--- a/gammapy/modeling/models/tests/test_spectral.py
+++ b/gammapy/modeling/models/tests/test_spectral.py
@@ -729,6 +729,20 @@ def test_template_spectral_model_compound():
     assert np.allclose(new_model(energy), 2 * values)
 
 
+def test_covariance_spectral_model_compound():
+    model = TEST_MODELS[2]["model"] + TEST_MODELS[1]["model"]
+    covar = np.eye(len(model.parameters))
+    covar[0, -1] = 1.0
+    covar[-1, 0] = 1.0
+    covar[0, 1] = 1.0
+    covar[-1, -3] = 1.0
+
+    model.covariance = covar
+    assert_allclose(model.covariance.data, covar)
+    assert_allclose(model.model1.covariance.data, covar[:3, :3])
+    assert_allclose(model.model2.covariance.data, covar[-3:, -3:])
+
+
 @requires_dependency("naima")
 class TestNaimaModel:
     # Used to test model value at 2 TeV
Repository: gammapy/gammapy
Pull request #5356 (2024-06-29 13:24:50)

Brought gammapy.visualization.plot_distribution in line with its documentation
Running 
```
fig, ax = plt.subplots(nrows=1, ncols=1)
gammapy.visualization.plot_distribution(ts_map, ax=ax)
```
results in an error, since gammapy.visualization.plot_distribution expects an array of axes in its current form, which does not fit the description, which explicitly allows singular axes objects and lists of axes objects:
```
Parameters
----------
wcs_map : `~gammapy.maps.WcsNDMap`
    A map that contains data to be plotted.
ax : `~matplotlib.axes.Axes` or list of `~matplotlib.axes.Axes`
    Axis object to plot on. If a list of Axis is provided it has to be the same length as the length of _map.data.
```
This pull request implements a fix, but maybe it is not in line with what the original author of the function intended (and still makes implicit assumptions about the input. As implemented, it expects an object or an array like object, on which np.array([obj]) can be called.)


MRegeard: @GerritRo, What you can do to make the code cleaner is to assign `axes = ax` before the first if statement (line 328). Then you do the if statement on axes. You also remove the `cells_in_grid` assignment in the if statment. This way you can completely remove the else statement and assign `cells_in_grid = axes.size` after (outside) of the instance check.
This should fix the issue and make the code cleaner at the same time. 
What you have to do anyway is to add a test (or modify the existing one) for this behaviour, as mentioned by @AtreyeeS. 

Patch:
diff --git a/gammapy/visualization/utils.py b/gammapy/visualization/utils.py
index 122dca4a8..464bc2d18 100644
--- a/gammapy/visualization/utils.py
+++ b/gammapy/visualization/utils.py
@@ -341,8 +341,8 @@ def plot_distribution(
         )
         cells_in_grid = rows * cols
     else:
-        axes = ax
-        cells_in_grid = len(ax.flatten())
+        axes = np.array([ax])
+        cells_in_grid = axes.size
 
     if not isinstance(axes, np.ndarray):
         axes = np.array([axes])


Test patch:
diff --git a/gammapy/visualization/tests/test_utils.py b/gammapy/visualization/tests/test_utils.py
index 57b1f551c..c423ac68f 100644
--- a/gammapy/visualization/tests/test_utils.py
+++ b/gammapy/visualization/tests/test_utils.py
@@ -138,3 +138,6 @@ def test_plot_distribution():
         axes, res = plot_distribution(
             wcs_map=map_, func="norm", kwargs_hist={"bins": 40}
         )
+
+        fig, ax = plt.subplots(nrows=1, ncols=1)
+        plot_distribution(map_empty, ax=ax)