Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct map evaluator to avoid memory overload #3529

Merged
merged 7 commits into from Oct 6, 2021

Conversation

registerrier
Copy link
Contributor

Description

This pull request corrects the MapEvaluator to avoid huge memory load when working with small extension sources. This was due to the fact that the upsampled geometry was not properly cutout. To alleviate this issue and solve the code duplication issue, all the oversampling and the cutout logic is left to SpatailModel.integrate_geom.

Initially, we kept the upsampled geom object on the MapEvaluator to avoid re-creating one each time a call to integrate_geom is made that requires oversampling the geometry. Creating WcsGeom does come at a cost in term of computing time.

Yet for now it seems simpler to fully rely on the SpatialModel.integrate_geom which consistently performs oversampling downsampling and reprojection. From a CPU time benchmark where the extension of PKS 2155 is fitted with HESS DR1, the current implementation is actually (twice) faster than the previous one.

A test is added that ensures that MapEvaluator.compute_flux_spatial yields correct result with relative accuracy of 1e-4 for varying source size. Note that this does not really test the very large memory usage. Is there a way to do so?

Dear reviewer

@registerrier registerrier self-assigned this Sep 28, 2021
@registerrier registerrier added this to In progress in gammapy.datasets via automation Sep 28, 2021
@registerrier registerrier added this to the v0.19 milestone Sep 28, 2021
@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

Merging #3529 (9ba57ee) into master (478fb9c) will decrease coverage by 0.01%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3529      +/-   ##
==========================================
- Coverage   93.86%   93.85%   -0.02%     
==========================================
  Files         157      157              
  Lines       19103    19111       +8     
==========================================
+ Hits        17931    17936       +5     
- Misses       1172     1175       +3     
Impacted Files Coverage Δ
gammapy/modeling/models/spatial.py 92.76% <66.66%> (-0.56%) ⬇️
gammapy/datasets/evaluator.py 93.75% <100.00%> (+0.02%) ⬆️

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 478fb9c...9ba57ee. Read the comment docs.

@registerrier registerrier merged commit c42b58e into gammapy:master Oct 6, 2021
gammapy.datasets automation moved this from In progress to Done Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

1 participant