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

Allow extrapolation only along spatial dimension in mask_safe_edisp #5271

Merged
merged 5 commits into from
May 27, 2024

Conversation

QRemy
Copy link
Contributor

@QRemy QRemy commented May 17, 2024

Support for extrapolation in interp_to_geom if geom is mask.
Allow extrapolation only along spatial dimension in mask_safe_edisp
to support case where mask_safe geom and irfs geom are different.

This a better solution to resolve the problem described in #5236.

@QRemy QRemy added backport-v1.0.x on-merge: backport to v1.0.x backport-v1.2.x on-merge: backport to v1.2.x bug labels May 17, 2024
Copy link

codecov bot commented May 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.46%. Comparing base (17caf4c) to head (f85fb7b).
Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5271      +/-   ##
==========================================
- Coverage   94.47%   94.46%   -0.01%     
==========================================
  Files         235      232       -3     
  Lines       35574    35636      +62     
==========================================
+ Hits        33608    33664      +56     
- Misses       1966     1972       +6     

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

@bkhelifi
Copy link
Member

bkhelifi commented May 22, 2024

Hi @QRemy,
I propose to add in the docstring of interp_to_geom' something like "If None, values outside the domain are extrapolated", as made in interp_by_pix'. Except that, it seems ok to me

Astro-Kirsty
Astro-Kirsty previously approved these changes May 22, 2024
Copy link
Member

@Astro-Kirsty Astro-Kirsty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @QRemy. This is a nice solution.

QRemy added 4 commits May 22, 2024 13:17
Signed-off-by: Quentin Remy <quentin.remy@mpi-hd.mpg.de>
Signed-off-by: Quentin Remy <quentin.remy@mpi-hd.mpg.de>
Signed-off-by: Quentin Remy <quentin.remy@mpi-hd.mpg.de>
Signed-off-by: Quentin Remy <quentin.remy@mpi-hd.mpg.de>
Copy link
Member

@bkhelifi bkhelifi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks @QRemy

@QRemy QRemy merged commit 6d5dbc6 into gammapy:main May 27, 2024
16 of 17 checks passed
Copy link

lumberbot-app bot commented May 27, 2024

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v1.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 6d5dbc66b87564694d327598d9fa647b883ec5e9
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #5271: Allow extrapolation only along spatial dimension in `mask_safe_edisp`'
  1. Push to a named branch:
git push YOURFORK v1.0.x:auto-backport-of-pr-5271-on-v1.0.x
  1. Create a PR against branch v1.0.x, I would have named this PR:

"Backport PR #5271 on branch v1.0.x (Allow extrapolation only along spatial dimension in mask_safe_edisp)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

meeseeksmachine pushed a commit to meeseeksmachine/gammapy that referenced this pull request May 27, 2024
QRemy added a commit that referenced this pull request May 28, 2024
…1-on-v1.2.x

Backport PR #5271 on branch v1.2.x (Allow extrapolation only along spatial dimension in `mask_safe_edisp`)
QRemy added a commit that referenced this pull request May 31, 2024
…n `mask_safe_edisp`

Signed-off-by: Quentin Remy <quentin.remy@mpi-hd.mpg.de>
QRemy added a commit that referenced this pull request May 31, 2024
Backport PR #5271: Allow extrapolation only along spatial dimension in `mask_safe_edisp`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v1.0.x on-merge: backport to v1.0.x backport-v1.2.x on-merge: backport to v1.2.x bug Still Needs Manual Backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants