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

Use more points to find image bounds in moasics #382

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

svank
Copy link
Contributor

@svank svank commented Aug 26, 2023

I'm trying out reproject_and_coadd (thanks @Cadair!). I'm making composite images from two WISPR images, which is together a pretty wide FOV (~100 degrees across). One of the two images is convex in the output projection, so checking where the image's corners fall in the output image isn't enough to properly bound it.

I've changed the function to put ten points along each edge of the image and use those to find its bounds in the output image, rather than just checking the corners. For my use-case, ten points per edge works just as well as doing every edge pixel.

Looking at just the footprint of the reprojected images, it's the difference between this (note the left edge):
image
and this:
image

@codecov
Copy link

codecov bot commented Aug 26, 2023

Codecov Report

Merging #382 (6642ab6) into main (afca2f2) will decrease coverage by 0.39%.
Report is 18 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #382      +/-   ##
==========================================
- Coverage   93.86%   93.47%   -0.39%     
==========================================
  Files          24       25       +1     
  Lines         847      873      +26     
==========================================
+ Hits          795      816      +21     
- Misses         52       57       +5     
Files Changed Coverage Δ
reproject/mosaicking/coadd.py 92.68% <100.00%> (+1.25%) ⬆️

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@astrofrog
Copy link
Member

What about making this value configurable via a kwarg? 10 seems a sensible default though.

@svank
Copy link
Contributor Author

svank commented Aug 28, 2023

Then I would have to make a test. 😬 But are you aware of projections that would require a lot more points? I'm not sure I've seen any that get truly wild---the smooth convexness in these WISPR images is the worst I can think of, and it seems well-served by a modest number of points along the edge. And I'm not sure there's a use case for turning the number down---whether we're doing the four corners or 40 points along the edges, the computational cost here seems truly negligible compared to the actual reprojection. So my thinking was to avoid a proliferation of options since I can't think of a good use case. But I can add the option if you'd rather.

Come to think of it, 9 or 11 might be the better default, so there's a point at the middle of each edge---it seems easy to imagine that being the most extreme point in cases where there's not also rotation at play, so might as well make sure to include it.

@astrofrog
Copy link
Member

@svank - sounds good about avoiding a kwarg. I'll push a change to make this 11 and will merge.

@astrofrog astrofrog merged commit b4a0e78 into astropy:main Sep 7, 2023
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants