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

Improve UVW coordinate calculations #46

Merged
merged 3 commits into from
Dec 11, 2018
Merged

Improve UVW coordinate calculations #46

merged 3 commits into from
Dec 11, 2018

Conversation

bmerry
Copy link
Contributor

@bmerry bmerry commented Dec 3, 2018

See the comments added to the code for detail about why this approach
improves things. As a side benefit, the special case handling for the
poles can be removed. This will change the results at the poles, in that
orientation will be determined by the given RA.

The golden RA/DEC in the unit tests changed. These are just a paste of
what is coming out, rather than independently verified results.

See the comments added to the code for detail about why this approach
improves things. As a side benefit, the special case handling for the
poles can be removed. This will change the results at the poles, in that
orientation will be determined by the given RA.

The golden RA/DEC in the unit tests changed. These are just a paste of
what is coming out, rather than independently verified results.
@bmerry
Copy link
Contributor Author

bmerry commented Dec 3, 2018

@mauch FYI.

Copy link
Contributor

@ludwigschwardt ludwigschwardt left a comment

Choose a reason for hiding this comment

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

Great stuff!

It's interesting that you only need to fix the m direction / v vector via an offset target. The distortion you mention therefore probably keeps l and m pretty much orthogonal locally.

DiFX also introduces an l offset target to fix the u vector independently of v (instead of insisting that they are orthogonal by using a cross product). I wonder how big that effect is...

# conversion doesn't simply rotate the celestial sphere, but also
# distorts it, and so that introduced errors.
#
# To avoid issues close to the poles, we always offset towards the
Copy link
Contributor

Choose a reason for hiding this comment

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

Clever.

ra, dec = self.radec(None, antenna)
else:
ra, dec = self.radec(timestamp, antenna)
offset_sign = -1 if dec > 0 else -1
Copy link
Contributor

Choose a reason for hiding this comment

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

You got -1 twice...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, thanks! Will fix.

# To avoid issues close to the poles, we always offset towards the
# equator. We also can't offset by too little, as ephem uses only
# single precision and this method suffers from loss of precision.
# 0.03 was found by experimentation (albeit on a single data set) to
Copy link
Contributor

Choose a reason for hiding this comment

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

0.03 radians, I assume. That's about 2 degrees... Interesting that one can go that big. I guess it is a mild distortion to begin with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect we'd get slightly better results if we could go smaller, but then the floating point errors climb rapidly.

u_norm = np.sqrt(np.sum(u ** 2, axis=0))
# If the target is a celestial pole (so that w equals z or -z), u and v become degenerate
Copy link
Contributor

Choose a reason for hiding this comment

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

Good riddance :-)

@bmerry
Copy link
Contributor Author

bmerry commented Dec 10, 2018

DiFX also introduces an l offset target to fix the u vector independently of v (instead of insisting that they are orthogonal by using a cross product). I wonder how big that effect is...

That's a good point. A related issue is scaling - we just normalise the vector we get back, but a unit vector in geocentric coordinates won't be a unit long in topocentric coordinates. But I think that locally the effects scale with β² rather than β (v/c ~= 1e-4) so are too small to worry about.

It doesn't affect the sign of the result, just leads to stability
problems within a few degrees of the south pole.
The previous bug-fix on whether to displace north or south caused the
results to change by enough to throw off the unit tests (a few ppm).
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.

None yet

2 participants