Repository: gammapy/gammapy
Pull request #5747 (2025-04-02 13:03:09)

Manually set FoVBackgroundModel name
<!-- These are hidden commments. You can leave them or delete them. -->

<!-- Don't be shy! Github issues or pull requests are welcome any time. -->
<!-- But if you first want to chat, use the Gammapy Slack. -->
<!-- See https://gammapy.org/contact.html -->

**Description**
<!-- What is the motivation for this pull request? -->
<!-- Briefly: what changes are done here? -->
<!-- If this is to address a Github issue, mention its number and use the specific keywords to create a link -->
<!-- See docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue -->
<!-- Example: This PR is to resolve #42 -->

This pull requests adds a "name" parameter to the init of the FoVBackgroundModel. If none is passed, the previous default one is used

**Dear reviewer**
<!-- Let the reviewer and Gammapy team know what you want: -->
<!-- * Is this ready for review? Or is it work in progress and you want some feedback? -->
<!-- * Do you want to go through review here? Or if someone should just finish this up and merge it? -->
<!-- Do you have any specific questions, e.g. about API or implementation? -->
<!-- Do you include a test executing the new code you're adding (to make sure it runs)? -->
<!-- Do you include some documentation? Is it needed? -->
This should be similar enough to the SkyModel and TemplateNPredModel. I checked if anything relies on the name exactly being the first dataset + "-bkg" and did not find anything, if I am mistaken please let me know :) 



Patch:
diff --git a/gammapy/datasets/map.py b/gammapy/datasets/map.py
index ef59ce0c6..6795323c3 100644
--- a/gammapy/datasets/map.py
+++ b/gammapy/datasets/map.py
@@ -586,10 +586,8 @@ class MapDataset(Dataset):
     # TODO: keep or remove?
     @property
     def background_model(self):
-        try:
-            return self.models[f"{self.name}-bkg"]
-        except (ValueError, TypeError):
-            pass
+        if self.models and self.name in self.models.background_models.keys():
+            return self.models[self.models.background_models[self.name]]
 
     def __str__(self):
         str_ = f"{self.__class__.__name__}\n"
diff --git a/gammapy/modeling/models/core.py b/gammapy/modeling/models/core.py
index dbf8828fe..459cb5332 100644
--- a/gammapy/modeling/models/core.py
+++ b/gammapy/modeling/models/core.py
@@ -110,6 +110,36 @@ def _check_name_unique(model, names):
     return
 
 
+def _check_fov_background_models(models):
+    """
+    Checks if a maximum of one `~gammapy.modeling.models.FoVBackgroundModel` is assigned to dataset
+    and returns a dictionnary mapping `dataset_name` to the background model name.
+
+    Parameters
+    ----------
+    models : `~gammapy.modeling.models.Models`
+        List of Models
+
+    Returns:
+    --------
+    bkg_model_mapping : dict
+        Dictionary mapping dataset name to `~gammapy.modeling.models.FoVBackgroundModel` name.
+    """
+    from . import FoVBackgroundModel
+
+    bkg_model_mapping = {}
+    for model in models:
+        if isinstance(model, FoVBackgroundModel):
+            for n in model.datasets_names:
+                if n not in bkg_model_mapping.keys():
+                    bkg_model_mapping[n] = model.name
+                else:
+                    raise ValueError(
+                        f"Only one FoVBackgroundModel per Dataset is permitted - already got one for {n}"
+                    )
+    return bkg_model_mapping
+
+
 def _write_models(
     models,
     path,
@@ -430,7 +460,10 @@ class DatasetModels(collections.abc.Sequence, CovarianceMixin):
             _check_name_unique(model, names=unique_names)
             unique_names.append(model.name)
 
+        self._background_models = _check_fov_background_models(models)
+
         self._models = models
+
         self._covar_file = None
 
         self._covariance = Covariance(self.parameters)
@@ -460,6 +493,11 @@ class DatasetModels(collections.abc.Sequence, CovarianceMixin):
         """List of model names."""
         return [m.name for m in self._models]
 
+    @property
+    def background_models(self):
+        """Dictionnary mapping of dataset names with their associated `~gammapy.modeling.models.FoVBackgroundModel` names."""
+        return self._background_models
+
     @classmethod
     def read(cls, filename, checksum=False):
         """Read from YAML file.
@@ -1256,6 +1294,7 @@ class Models(DatasetModels, collections.abc.MutableSequence):
 
     def __delitem__(self, key):
         del self._models[self.index(key)]
+        self._background_models = _check_fov_background_models(self._models)
 
     def __setitem__(self, key, model):
         from gammapy.modeling.models import (
@@ -1275,6 +1314,7 @@ class Models(DatasetModels, collections.abc.MutableSequence):
     def insert(self, idx, model):
         _check_name_unique(model, self.names)
         self._models.insert(idx, model)
+        self._background_models = _check_fov_background_models(self._models)
 
     def set_prior(self, parameters, priors):
         for parameter, prior in zip(parameters, priors):
diff --git a/gammapy/modeling/models/cube.py b/gammapy/modeling/models/cube.py
index 627d534d1..3a4c84bb7 100644
--- a/gammapy/modeling/models/cube.py
+++ b/gammapy/modeling/models/cube.py
@@ -638,6 +638,12 @@ class FoVBackgroundModel(ModelBase):
     spatial_model : `~gammapy.modeling.models.SpatialModel`, Optional
         Unitless Spatial model (unit is dropped on evaluation if defined).
         Default is None.
+    covariance_data : `~numpy.ndarray`, optional
+        Covariance data array.
+        Default is None.
+    name : str, optional
+        Name of the created object.
+        Default is None and the name is generated automatically.
     """
 
     tag = ["FoVBackgroundModel", "fov-bkg"]
@@ -648,6 +654,7 @@ class FoVBackgroundModel(ModelBase):
         spectral_model=None,
         spatial_model=None,
         covariance_data=None,
+        name=None,
     ):
         # TODO: remove this in v2.0
         if isinstance(dataset_name, SpectralModel):
@@ -667,7 +674,10 @@ class FoVBackgroundModel(ModelBase):
 
         if not spectral_model.is_norm_spectral_model:
             raise ValueError("A norm spectral model is required.")
-
+        if name is not None:
+            self._name = make_name(name)
+        else:
+            self._name = self.datasets_names[0] + "-bkg"
         self._spatial_model = spatial_model
         self._spectral_model = spectral_model
         super().__init__(covariance_data=covariance_data)
@@ -695,7 +705,7 @@ class FoVBackgroundModel(ModelBase):
     @property
     def name(self):
         """Model name."""
-        return self.datasets_names[0] + "-bkg"
+        return self._name
 
     @property
     def parameters(self):


Test patch:
diff --git a/gammapy/modeling/models/tests/test_core.py b/gammapy/modeling/models/tests/test_core.py
index 682250c64..83b2a9e00 100644
--- a/gammapy/modeling/models/tests/test_core.py
+++ b/gammapy/modeling/models/tests/test_core.py
@@ -15,6 +15,7 @@ from gammapy.modeling.models import (
     PointSpatialModel,
     PowerLawSpectralModel,
     SkyModel,
+    FoVBackgroundModel,
 )
 from gammapy.utils.testing import mpl_plot_check, requires_data
 
@@ -349,3 +350,13 @@ def test_add_not_unique_models():
         match="Model names must be unique. Models named 'source1' are duplicated.",
     ):
         models1.extend(models2)
+
+
+def test_two_fov_bkg_models_single_dataset():
+    fov1 = FoVBackgroundModel(dataset_name="ds1", name="ds1-1")
+    fov2 = FoVBackgroundModel(dataset_name="ds1", name="ds1-2")
+    with pytest.raises(
+        ValueError,
+        match="Only one FoVBackgroundModel per Dataset is permitted - already got one for ds1",
+    ):
+        Models([fov1, fov2])
diff --git a/gammapy/modeling/models/tests/test_cube.py b/gammapy/modeling/models/tests/test_cube.py
index 879c914eb..d68d22e73 100644
--- a/gammapy/modeling/models/tests/test_cube.py
+++ b/gammapy/modeling/models/tests/test_cube.py
@@ -1109,3 +1109,10 @@ def test_piecewise_spatial_model_background(background):
     assert isinstance(copied.spatial_model, PiecewiseNormSpatialModel)
 
     assert "Spatial model type" in copied.__str__()
+
+
+def test_naming_fov_background_model():
+    fov_default = FoVBackgroundModel(dataset_name="one", name=None)
+    assert fov_default.name == "one-bkg"
+    fov_named = FoVBackgroundModel(dataset_name="two", name="custom_name")
+    assert fov_named.name == "custom_name"