Reduce memory usage of MapEvaluator

[QRemy]

Avoid caching exposure cutout on MapEvaluator. The cutout creates a new array in memory for each source so this does not scale well if the cutout region and the number of sources is large, while the cutout is very fast to compute.

[QRemy]

reference

this PR

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (ca22055) 75.69% compared to head (8f708ed) 75.69%.

Files	Patch %	Lines
gammapy/datasets/evaluator.py	95.65%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4989   +/-   ##
=======================================
  Coverage   75.69%   75.69%           
=======================================
  Files         228      228           
  Lines       33841    33851   +10     
=======================================
+ Hits        25616    25624    +8     
- Misses       8225     8227    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adonath]

Are you sure the difference comes from the exposure? The cutout should only return a view and not copy any values. However the cutout geom re-computes the array of coordinates. I would presume the memory usage comes from this.

[QRemy]

Are you sure the difference comes from the exposure? The cutout should only return a view and not copy any values. However the cutout geom re-computes the array of coordinates. I would presume the memory usage comes from this.

from what I understand the cutout creates a new map with a new data array :

gammapy/gammapy/maps/wcs/ndmap.py

Lines 992 to 995 in 888905e

	data = np.zeros(shape=geom_cutout.data_shape, dtype=self.data.dtype)
	data[cutout_slices] = self.data[parent_slices]
	return self._init_copy(geom=geom_cutout, data=data)

and the coordinates are not re-computed as they are cached only one time with the lru_cache of the original geom

[QRemy]

I updated the reference plot from my previous comment (it was wrong because I forgot to cherry pick one of the other memory patches in my reference branch).

[adonath]

I updated the reference plot from my previous #4989 (comment) (it was wrong because I forgot to cherry pick one of the other memory patches in my reference branch).

Thanks, that's even worse. There is a memory leak...

[adonath]

and the coordinates are not re-computed as they are cached only one time with the lru_cache of the original geom

Yes, but no...

As we create a cutout geom for each source, it is a new object. The first time we access the coordinates on the cutout geom, they are re-computed and cached. Worst case, when there are a lot of source and large support, we duplicate all the coordination information. In this case it might be better to create a cutout from the original larger coordinate arrays. But this highly depends on the analysis scenario, for few sources computing the coordinates on the cutouts is probably much better.

from what I understand the cutout creates a new map with a new data array :

Yes, you are right. I might be worth to only work with view and give up on the not fully contained cutouts, but trim instead.

[QRemy]

Thanks, that's even worse. There is a memory leak...

For this test I have 32 sources, and each new peak corresponds to the npred computation of a source

[QRemy]

As we create a cutout geom for each source, it is a new object. The first time we access the coordinates on the cutout geom, they are re-computed and cached. Worst case, when there are a lot of source and large support, we duplicate all the coordination information. In this case it might be better to create a cutout from the original larger coordinate arrays. But this highly depends on the analysis scenario, for few sources computing the coordinates on the cutouts is probably much better.

Right, at least we could save memory on coordinates caching by returning a 2D meshgrid for lon, lat if the geom is regular and a 1d array for the axes instead of a ND meshgrid for each axis. I will try that in another PR.

[adonath]

For this test I have 32 sources, and each new peak corresponds to the npred computation of a source

Ok ,thanks for clarifying. It thought it was multiple npred evaluations. But it is only one with 32 sources.

[registerrier]

Thanks @QRemy . This looks good. No comment from my side.