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

Further improve model region / mask selection #3215

Merged
merged 9 commits into from Feb 3, 2021

Conversation

adonath
Copy link
Member

@adonath adonath commented Feb 2, 2021

Description
This pull request is a follow-up to #3169. It further breaks down the functionality like so:

  • Introduce Map.mask_nearest_position(), which returns the nearest position inside the mask to given position
  • Move the selection of the nearest IRF to IRFMap. Right now it was using the mask_safe_psf for PSF as EDisp, however it's not guaranteed that those agree. For this is introduce a IRFMap.mask_safe_irf, which derives the mask from the data. however we might think about moving the mask safe for the IRF to the IRFMap completely, this would be more than convenient for stacking as well...
  • Test wise I introduced RegionGeom.from_regions(), which creates a RegionGeom with a CompoundSkyRegion from a list of regions, while working on this I think we should clarify the handling of CompoundRegion vs. a list of Region, everywhere in Gammapy. I added it as a discussion point on the agenda for the dev call on Friday.

Dear reviewer

@adonath adonath changed the title Cleanup model region / mask selection Further improve model region / mask selection Feb 2, 2021
@adonath adonath self-assigned this Feb 2, 2021
@adonath adonath added this to the v0.19 milestone Feb 2, 2021
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #3215 (fb3e2ba) into master (39133de) will decrease coverage by 0.03%.
The diff coverage is 91.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3215      +/-   ##
==========================================
- Coverage   93.91%   93.87%   -0.04%     
==========================================
  Files         144      144              
  Lines       17441    17467      +26     
==========================================
+ Hits        16380    16398      +18     
- Misses       1061     1069       +8     
Impacted Files Coverage Δ
gammapy/datasets/simulate.py 99.40% <ø> (ø)
gammapy/estimators/ts_map.py 97.66% <ø> (ø)
gammapy/maps/wcsnd.py 95.31% <75.00%> (-0.79%) ⬇️
gammapy/modeling/models/cube.py 87.98% <81.25%> (-0.51%) ⬇️
gammapy/maps/core.py 86.56% <87.50%> (+0.01%) ⬆️
gammapy/maps/region.py 89.61% <88.88%> (-0.07%) ⬇️
gammapy/datasets/map.py 94.41% <100.00%> (-0.14%) ⬇️
gammapy/irf/core.py 91.10% <100.00%> (+0.35%) ⬆️
gammapy/irf/edisp_map.py 96.05% <100.00%> (+0.05%) ⬆️
gammapy/irf/psf/psf_map.py 96.62% <100.00%> (+0.02%) ⬆️
... and 3 more

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 39133de...fb3e2ba. Read the comment docs.

@adonath adonath merged commit 90a3464 into gammapy:master Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant