Add .select_region() and .select_mask() methods to Models

[QRemy]

This PR add models management methods to Models :

• .select_region(regions) : Select skymodels with center position contained within a given region
• .contribute(mask, psf): check if a model contribute within a mask map

[codecov]

Codecov Report

Merging #3169 (e70af52) into master (c651f4d) will increase coverage by 0.00%.
The diff coverage is 93.80%.

@@           Coverage Diff           @@
##           master    #3169   +/-   ##
=======================================
  Coverage   93.00%   93.00%           
=======================================
  Files         147      147           
  Lines       17689    17780   +91     
=======================================
+ Hits        16452    16537   +85     
- Misses       1237     1243    +6     

Impacted Files	Coverage Δ
gammapy/datasets/simulate.py	99.40% <ø> (ø)
gammapy/estimators/ts_map.py	97.66% <ø> (ø)
gammapy/maps/core.py	86.51% <ø> (ø)
gammapy/maps/wcs.py	96.86% <50.00%> (-0.23%)	⬇️
gammapy/modeling/models/spatial.py	95.12% <69.23%> (-0.95%)	⬇️
gammapy/modeling/models/cube.py	88.48% <95.45%> (+0.35%)	⬆️
gammapy/maps/wcsnd.py	96.09% <97.14%> (+0.30%)	⬆️
gammapy/datasets/map.py	94.59% <100.00%> (+0.02%)	⬆️
gammapy/modeling/models/core.py	96.66% <100.00%> (+0.19%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c651f4d...e70af52. Read the comment docs.

[adonath]

Thanks @QRemy! Sorry for the delay here, I have left a few comment to address.

[adonath]

Sorry @QRemy, for the delay on this, it took me a while to find the time to think about the code structure again. My goal is always to break the functionality down into as many useful sub-functions as possible, that we can re-use in other places too. So here comes my final proposal:

• Let's introduce SpatialModel.evaluation_region, this can probably just rely on .to_region() or the other way around. This allows to handle non-circular regions as well later. The difference is just the size of the region...maybe adding Models.evaluation_regions is useful as well.
• In addition we introduce Map.contains_region(regions=regions), which checks whether a Region is contained in a mask. Ideally this can handle PixRegions as well. The main point here, is that the method can be reused in the ReflectedRegionsFinder, where exactly the same functionality is needed (I can do the refactoring later...). A list of regions should be supported as well and return a boolean array.
• Using these methods the SkyModel.contributes() becomes very simple, possibly it could be replaced by mask.dilate(margin=).contains_region(model.evaluation_region) completely in the MapEvaluator as well
• The Models.select_region() is fine as is
• Models.select_mask(), also relies on model.evaluation_region and .contains_region()

I think this way we have consistency between the selection in MapEvaluator and Models.select_mask(), as well as additional functionality, that can bis used in the other places as well. Does this sound reasonable to you?

[QRemy]

Sounds good, I just have some doubts about mask.dilate(margin=) it may to have extend the geom size by the margin but that will cause problems elsewhere (could introduce keepdims=True option by default), and Map.contains_region() should take into account the values of the mask but that make not much sense for non-boolean map.

[adonath]

Thanks @QRemy, those are good points:

• I think this case can be handled with a call to Map.pad() beforehand or having different modes in Map.binary_dilate()
• I agree the .contains_region() is only well defined for a boolean mask, we could name it .mask_contians_region() for clarity and raise an error if the map doesn't contain boolean values, just as we do for .binary_dilate(), at some point I added a flag called Map.is_mask...

[adonath]

Thanks a lot @QRemy some of the methods might need a unit test, but this can be done later...