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

Fix symmetry issue in solid angle calculation for WcsGeom #2035

Merged
merged 7 commits into from Feb 18, 2019

Conversation

Projects
None yet
3 participants
@watsonjj
Copy link
Contributor

watsonjj commented Feb 15, 2019

Currently the solid angle calculation for a WcsGeom is not symmetric about the origin. This is a result of calculating the separation along the bin edge instead of along the bin centre.

@watsonjj

This comment has been minimized.

Copy link
Contributor Author

watsonjj commented Feb 15, 2019

I will explain the problem, and what is causing it:

image

In the image above I have created a Wcs Map centred on the origin with three bins in each axis. Currently the dx values for the solid angle calculation are calculated along the bottom of the pixels as shown in the above figure. Due to the great circle separations of the coordinate frame, the separation between the lon coordinates of the pixel edges reduces as you increase/decrease in lat. As the bottom of the pixels is symmetric for the top two pixels, they have the same x separation. They will therefore result in the same solid angle. Whereas the bottom pixel will have a different solid angle value, as x2 is measured at a further distance along lat from the origin.

The problem I see with this is the solid angle measured for each pixel is not symmetric about the origin. I would have expected the top and bottom pixel to have the same solid angle value, as they both have the same distance along the lat axis.

image

If one instead measures the separation along the centre of the pixel, as show above, then the calculation is symmetric about the origin, and the solid angles come out as expected.

image

The above image shows the expected area distribution across pixels.

@adonath adonath requested a review from registerrier Feb 15, 2019

@registerrier registerrier added the bug label Feb 15, 2019

@registerrier registerrier added this to the 0.11 milestone Feb 15, 2019

@registerrier
Copy link
Contributor

registerrier left a comment

Looks good. It is nice to have spotted that.

How significant is the change in the solid angle results for typical test cases we have implemented. I imagine there should be some change in many gammapy.cube tests, no?

ylo_xlo = lon[..., :-1, :-1], lat[..., :-1, :-1]
ylo_xhi = lon[..., :-1, 1:], lat[..., :-1, 1:]
yhi_xlo = lon[..., 1:, :-1], lat[..., 1:, :-1]
# TODO: Calculate actual solid angle between two great circles?

This comment has been minimized.

Copy link
@registerrier

registerrier Feb 15, 2019

Contributor

A priori I think that one can use great circle only for very specific WCS projections, e.g. for the Plate-Carrée or CAR projection. But for most other projections the axes do not follow great circles e.g. Hammer-Aitoff AIT or tangential TAN.

So it is possible to use exact formulae for some specific projections, but not in general.

This comment has been minimized.

Copy link
@watsonjj

watsonjj Feb 15, 2019

Author Contributor

Okay. Shall I remove this TODO?

@watsonjj

This comment has been minimized.

Copy link
Contributor Author

watsonjj commented Feb 15, 2019

How significant is the change in the solid angle results for typical test cases we have implemented. I imagine there should be some change in many gammapy.cube tests, no?

I have committed the changes required for other tests that had a small (on the order of 0.01%) change in the fixed values

@registerrier
Copy link
Contributor

registerrier left a comment

Thanks @watsonjj !

@adonath
Copy link
Member

adonath left a comment

I added two references for the TODO comment on improving the solid angle computation. Thanks @watsonjj for spotting and fixing this issue!

@adonath adonath self-assigned this Feb 18, 2019

@watsonjj watsonjj force-pushed the watsonjj:symmetric_solid_angle branch from ad3f270 to f502df7 Feb 18, 2019

@adonath
Copy link
Member

adonath left a comment

Thanks for fixing the merge conflicts, @watsonjj!

@adonath adonath merged commit 8d2498c into gammapy:master Feb 18, 2019

3 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: 1 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@adonath adonath changed the title Symmetric solid angle calculation for WcsGeom Fix symmetry issue in solid angle calculation for WcsGeom Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.