skyfielders / python-skyfield

Elegant astronomy for Python
MIT License
1.41k stars 212 forks source link

Polar Motion #372

Closed mkbrewer closed 3 years ago

mkbrewer commented 4 years ago

I am trying to track down what I think may be a bug in SOFA regarding polar motion. In their function Pvtob(), they do the standard rotation of the Earth by xp around the y axis and then yp around the x axis. But then, when calculating the observed azimuth and elevation, they do another rotation of the topocentric CIRS apparent position of the source. This second rotation looks very much like the alternate approximation for polar motion described in equation 3.27-3 on page 140 of the 1992 edition of the Explanatory Supplement to the Astronomical Almanac, just missing a factor of tangent latitude. I have never heard of anyone applying both of these before.

I wrote my own solar system ephemeris program using SOFA. When I compare my output without polar motion to JPL Horizons, I see an offset of about 5 mas in declination and 9 meters in distance over the course of a day for the Moon. The azimuth and elevation, however, are good to the 4 decimal places printed by Horizons. When I compare my output with polar motion to JPL Horizons, the declination offset is reduced to 0.5 mas and the distance offset is reduced to 0.8 meters. The azimuth and elevation, however, are now off by about +/-1 arcsecond. If I disable the second rotation this azimuth and elevation offset disappears.

As a second check, I compared my data to Skyfield. Without polar motion in either, I am seeing an offset of about 0.5 mas in Dec and 1.5 mas in RA, a +/-0.8 meter offset in distance, about +/-17 mas in azimuth and about -3 to +9 mas in elevation. When I add polar motion to my data, but not to Skyfield, the Dec offset increases to about 5 mas, the RA offset ranges from -3.7 mas to +1.8 mas and the distance offset ranges from -8 to +2 meters. The azimuth and elevation offsets are greater than +/-1 arcseconds again consistent with Horizons. Then I tried adding polar motion to Skyfield also. This resulted in a Dec offset ranging from -11 to +0.f mas, a RA offset ranging from -5.5 to -0.3 mas, a distance offset of -4 to +5 meters and the same +/-1 arcsecond offset in azimuth and elevation.

I find it odd that adding polar motion to Skyfield resulted in no improvement in the RA and distance offsets and actually made the Dec offset worse. So my question is: is there a known problem with polar motion offsets in Skyfield?

btw, I used the same DE403 ephemeris that Horizion uses in both my program and Skyfield.

mkbrewer commented 3 years ago

Yes, the NOVAS function terra() ignores polar motion unless the observer's longitude and latitude have been transformed to the TIRS in the manner described in equations 6.20 and 6.21 in USNO Circular 179. They acknowledge that in Note 2:

      2. This function ignores polar motion, unless the
      observer's longitude and latitude have been corrected for it,
      and variation in the length of day (angular velocity of earth).

There are, however, a few problems with this approach. First of all it operates on the observer's geodetic position rather than the observer's geocentric position, thus ignoring the flattening of the ellipsoid. This creates a minor error of the order of tens of micro-arcseconds. A much more significant error occurs if this approach is continued down the line. When converting from equatorial to horizontal coordinates, this approach completely ignores the angle CIP->observer->ITRS pole. Since, from the point of view of the observer the origin of azimuth is the ITRS pole, this requires a correction to azimuth which can be quite significant at high latitudes.

The approach that you have taken here is much better. This is the rigorous approach that is used by SOFA. The resulting GCRS position and velocity of the observer are absolutely correct for use in calculating light time delay, aberration and gravitational light deflection for topocentric observers.

Regarding testing, perhaps it would be best for you to just test the new version yourself using the script and data from my program here:

skyfield-test.zip

Of course, if you really want me to do the testing. I can try what you suggest. Since I've never done that before, I do have a question. Does that command do the necessary compilation and overwrite my current version located in /usr/local/lib/ assuming that I use sudo?

mkbrewer commented 3 years ago

If you are willing run the script in skyfield-test.zip and post the output here, I can easily create new plots from your data. In fact, that may be the best approach.

brandon-rhodes commented 3 years ago

After tweaking your script to use the de430t.bsp file I have here on my laptop, I receive this output:

test.output.txt

mkbrewer commented 3 years ago

Sorry, I got sidetracked by the people that actually pay me to work for them. We are now halfway there. The residuals in RA/Dec and moon distance are now exactly the same as they were in the no polar motion case, but the residuals in Az/El are still about the same. Do you follow the same method that NOVAS uses in equ2hor()?

brandon-rhodes commented 3 years ago

The residuals in RA/Dec and moon distance are now exactly the same as they were in the no polar motion case…

Amazing! It worked!

…the residuals in Az/El are still about the same. Do you follow the same method that NOVAS uses in equ2hor()?

Not yet — that’s my next step, now that I have confidence in my first result. I suspect that all I need to do is apply xp and yp in _altaz_rotation() and we will have a match for Az/El, but we’ll see.

And then I’ll need to write a bit of infrastructure. I think my scheme will be:

  1. Teach the Timescale to load xp and yp arrays from finals2000A.all.
  2. Provide a Timescale array of rotation matrices (named what? t.polar_motion? t.xp_yp?) that it builds using interpolated values for xp and yp that it builds from the arrays.
  3. Teach the Topos to use those rotation matrices automatically in _at() and _altaz_rotation() where currently it only knows how to take and use single static xp and yp values that have been provided manually.
  4. I will keep the Topos xp and yp around for compatibility with any existing scripts (assuming that's worth it? They were being applied wrong so they weren't helping anybody. Maybe I should ignore them to keep the code simpler, it would only make existing code more accurate!) and fall back to using them if they’re provided.

Then my big choice will be: does polar motion always get applied automatically. My guess is that I leave it optional for a version or two until I'm confident that it's working for everyone, then turn it on by default. But maybe I'll get really confident and just flip the switch immediately. We'll see.

Thanks for providing several months of patient prodding until I got the time to tackle this, it’s satisfying to finally be able to make progress on it.

mkbrewer commented 3 years ago

Assuming that you do automate polar motion, please provide either a method or attributes so that the user can retrieve the values of xp and yp for any given time. This is important to me as I use these in my program to do comparisons using the same EOPs that were used originally.

brandon-rhodes commented 3 years ago

please provide either a method or attribute so that the user can retrieve the values of xp and yp for any given time

That's a great idea! I'll make sure the mechanism is exposed.

mkbrewer commented 3 years ago

I see a new commit here. I'd very much like to see either the output data from my script or a plot of the Az/El residuals wrt my data.

Also, one minor quibble. To be strictly rigorous, in _at() the observer's velocity should be rotated by the wobble matrix or recomputed from the rate of change of the observer's TIRS position. Of course, if this is only used to compute diurnal aberration, being a minor correction to a minor correction, this can be treated as an ignorable second order effect.

brandon-rhodes commented 3 years ago

Yes, a new commit! It was a late-night commit so I didn’t get the chance to run any numbers; we’ll see if I can spare some time this weekend. I should in the meantime answer your earlier question:

Does that command do the necessary compilation and overwrite my current version located in /usr/local/lib/ assuming that I use sudo?

It should put Skyfield in the same place that pip previously did, yes. Skyfield requires no compilation, so all that happens is that the files are copied into place on top of the old ones. If you ever want to revert back to the latest release, I think these commands will do it:

pip uninstall skyfield
pip install skyfield
brandon-rhodes commented 3 years ago

@mkbrewer — I have disabled the old xp and yp arguments to Topos since (a) they never worked and only made calculations less accurate, so no current code successfully depends on their doing the right thing and will in general be more accurate if they're ignored; (b) they are actually functions of time, and a Topos does not have a static time of its own; and (c) they are not particular to any one Topos — different Earth locations don't experience different xp and yp — and so they don't belong on the Topos object.

The Timescale object is now where the time-dependent concept of polar motion lives. A timescale can now have a polar motion table attached to activate the feature. For a fixed value of xp and yp that apply to all calculations, one can simply do:

ts.polar_motion_table = [0.0], [xpole], [ypole]

I have added that line to your skyfield_topo_test.py and got the following output:

test.output.txt

Now that the implementation is becoming mature, I have gone ahead and also applied polar motion to the velocity.

Next I will be wiring things up so that finals2000A.all can supply the polar motion table.

Do you have any opinion on what should happen out past the ends of finals2000A.all for polar motion?

  1. The initial and final values could be carried out infinitely into all history and all of the future.
  2. The values of x and y could revert to 0 immediately before and after the first and final dates in finals2000A.all, but this would create an unhappy discontinuity in graphs, not to mention a bit of violence to derivatives.
  3. Or, an extra point could be added to the beginning and end of the table, taking the angles gently to 0.0 over some period of hours, days, or weeks. I would welcome any ideas on how quickly they should revert.
mkbrewer commented 3 years ago

Happy to see that Az/El is working well now also. The residuals are nearly the same as in the no polar motion case. I'll let the Astopy people know that you have fixed this so that their tests will stop failing with the next release of Skyfield. Now I'm going to have to talk to Jon Giorgini at JPL to figure out why JPL Horizons is so far off in Az/El. Regarding long term polar motion, historical data going back to 1846 are available here: https://www.iers.org/IERS/EN/DataProducts/EarthOrientationData/eop.html. I don't know about future predictions though. I suppose you also have that issue with DUT1, how do you handle that? I'll leave this issue open for now. Please let me know when your treatment of this is finalized.

brandon-rhodes commented 3 years ago

The residuals are nearly the same as in the no polar motion case.

Wonderful! I am glad our separate routines now agree about polar motion. (Though I am always still interested in the lingering residuals, and learning their source.)

Now I'm going to have to talk to Jon Giorgini at JPL to figure out why JPL Horizons is so far off in Az/El.

I'll be interested to hear if you learn what HORIZONS does differently. Also, your original question was about sofa — did you demonstrate to your satisfaction that sofa was applying polar motion twice?

Regarding long term polar motion, historical data going back to 1846 are available here…

Oh, wow, thank you, I had not noticed that data file! I'll add a TODO item for getting that added to Skyfield.

I suppose you also have that issue with DUT1, how do you handle that?

Right now I use the old tables from Morrison and Stephenson (2004), but last month I read Morrison, Stephenson, and Hohenkerk (2016) and am looking forward to teaching Skyfield their revised 2018 splines for historic Earth rotation.

Please let me know when your treatment of this is finalized.

I will continue to report progress here.

mkbrewer commented 3 years ago

did you demonstrate to your satisfaction that sofa was applying polar motion twice?

Yes. And I figured out in the six months since I raised this issue that SOFA is correct in doing that. I have been doing this wrong for the last 25 years as I have just been using the formulas from the ES to handle polar motion. These just move the observer's location from the ITRS to the TIRS using an adjustment to the geodetic latitude and longitude of the observer as is also described in section 6.5.4 of USNO Circular 179. This comes very close to what you are doing in _at(), but it neglects transforming the source position vector back into the ITRS before rotating by ITRS longitude to form the hour angle as you are now doing in _altaz_rotation(). Instead, it just uses the TIRS LST derived from using the adjusted longitude of the observer. As I pointed out above, this ignores the angle CIP->observer->ITRS pole which results in an offset in azimuth. There is also an offset in elevation due to the use of the adjusted latitude.

Right now I use the old tables from Morrison and Stephenson (2004), but last month I read Morrison, Stephenson, and Hohenkerk (2016) and am looking forward to teaching Skyfield their revised 2018 splines for historic Earth rotation.

Well, my question there was mainly in regards to future predictions. Do these tables also go forward to say 2120?

brandon-rhodes commented 3 years ago

Yes. And I figured out in the six months since I raised this issue that SOFA is correct in doing that.

Oh! So they were not applying the correction twice in the same direction — they were doing it once when turning a lat/lon into a GCRS position, and then again when going back in the other direction to take an observed position back into the lat/lon’s local frame of reference. Understood.

From your comments a few months ago, though, is sofa doing it a little inconsistently — table-driven one time, but then with an approximate formula the other time, instead of using the same xp and yp both times?

Well, my question there was mainly in regards to future predictions. Do these tables also go forward to say 2120?

Oh — no, alas, they seem to all stop with the present, or else go 1 year into the future.

mkbrewer commented 3 years ago

From your comments a few months ago, though, is sofa doing it a little inconsistently — table-driven one time, but then with an approximate formula the other time, instead of using the same xp and yp both times?

You're going to need to point me to those comments. I can't find them. But, no, SOFA uses xp and yp consistently in both directions.

brandon-rhodes commented 3 years ago

You're going to need to point me to those comments. I can't find them.

At one point above, you had indicated of SOFA:

“In their function Pvtob(), they do the standard rotation of the Earth by xp around the y axis and then yp around the x axis. But then, when calculating the observed azimuth and elevation, they do another rotation of the topocentric CIRS apparent position of the source. This second rotation looks very much like the alternate approximation for polar motion described in equation 3.27-3 on page 140 of the 1992 edition of the Explanatory Supplement to the Astronomical Almanac, just missing a factor of tangent latitude. I have never heard of anyone applying both of these before.”

The description of an “alternate approximation for polar motion … missing a factor of tangent latitude” had sounded to me like polar motion numbers drawn from an equation rather than from an xp/yp table. (Lacking the 1992 edition, I never did go try to find the equation in print; I only have the Third Edition.)

mkbrewer commented 3 years ago

Oh, OK. That was my original post: https://github.com/skyfielders/python-skyfield/issues/372#issue-613644410 The way SOFA does the transformation to azimuth and elevation is quite confusing at first glance. They do this during setup, since xp and yp vary slowly with time:

/* Longitude with adjustment for TIO locator s'. */
   astrom->along = elong + sp;

/* Polar motion, rotated onto the local meridian. */
   sl = sin(astrom->along);
   cl = cos(astrom->along);
   astrom->xpl = xp*cl - yp*sl;
   astrom->ypl = xp*sl + yp*cl;

You may notice that xpl and ypl are quite similar to the equations in the ES or equations (6.21) and (6.20) in USNO Circular 179. This is what I was referring to. I normally call that function once a minute.

Then with the rapidly varying local earth rotation angle eral = era + astrom->along, they form the hour angle and then do this:

/* CIRS RA,Dec to Cartesian -HA,Dec. */
   iauS2c(ri-astrom->eral, di, v);
   x = v[0];
   y = v[1];
   z = v[2];

/* Polar motion. */
   xhd = x + astrom->xpl*z;
   yhd = y - astrom->ypl*z;
   zhd = z - astrom->xpl*x + astrom->ypl*y;

There ri and di are the CIRS right ascension and declination, iauS2c() just converts from spherical to Cartesian. I normally call the function that does this 200 times per second after interpolating the CIRS right ascension and declination which I calculate once per minute. Of course, for these comparisons, I just call both functions once per minute.

I eventually worked through that to prove that this is exactly the same as what you are doing, just a bit more efficient.

mkbrewer commented 3 years ago

Oh, I didn't notice this:

I never did go try to find the equation in print; I only have the Third Edition.

Having both, I can point you to the page. Equation (6.93) on page 242 of the third edition.

brandon-rhodes commented 3 years ago

Thanks for the page reference, the equations there match your description exactly!

I am thinking about doing a Skyfield release today, to get the new lunar_eclipses() routine out to users, and then continue to work on polar motion over the next couple of weeks — as there are several more steps necessary before I'll have the feature all wrapped up nicely for users and ready to document.

Right now, the x and y arguments to Topos that, it turns out, only ever made things worse, have been disabled; and if the user manually supplies a polar_motion_table for their timescale (as illustrated above), then it will be linearly interpolated for its x and y values in arcseconds. So though it won't be documented yet, you'll be able to use the new and better polar motion support if you'd like.

mkbrewer commented 3 years ago

I'm in no huge rush to use this new functionality. I've waited since May, so a couple more weeks isn't a problem.

JoshPaterson commented 3 years ago

@brandon-rhodes, you asked above:

Do you have any opinion on what should happen out past the ends of finals2000A.all for polar motion?

I'm not sure if this is helpful or not but Bulletin A from IERS (https://www.iers.org/IERS/EN/DataProducts/EarthOrientationData/eop.html) seems to contain formulas for predicting x and y past the end of the data in finals2000A.all.

mkbrewer commented 3 years ago

Yes, those are all I ever use for x, y and dut1 in my telescope tracking programs. But then, I'm a radio astronomer and I do update the formulas once a year. I am still curious as to how Skyfield currently handles dut1 for dates more than a year in the future.

mkbrewer commented 3 years ago

btw, After six months of working with them to help get the bugs out of their code, the latest data that I have from Astropy master tracks my program down to the last significant decimal (3.6 uas) in all quantities. They are currently forgoing checks against Skyfield due to this polar motion issue, so it'll be nice when this is finalized.

brandon-rhodes commented 3 years ago

seems to contain formulas for predicting x and y past the end of the data

Thanks, @JoshPaterson, I'll add that to my list of things to take a look at!

I am still curious as to how Skyfield currently handles dut1 for dates more than a year in the future.

It assumes no further UTC leap seconds, and once it runs out of future ∆T predictions it falls back on the parabola from Morrison and Stephenson 2004.

They are currently forgoing checks against Skyfield due to this polar motion issue, so it'll be nice when this is finalized.

What do you do in your own library out past the end of the polar xy table, @mkbrewer? Fall back on formula? Have the value gradually regress to zero? Or are you typically not targeting dates that far in the future?

mkbrewer commented 3 years ago

I don't deal with that at all. Currently, I just use starting and ending values for the EOPs and linearly interpolate between them. We are just using the one script for 24 hours on the Moon starting at April 6th 2017, 0 UTC, so when I get data from them, I just enter the starting and ending values of x, y and dut1 into my program to run the comparison.

mkbrewer commented 3 years ago

This is why I need to be able to retrieve x and y from Skyfield, so that I can use them also. I can already get dut1.

brandon-rhodes commented 3 years ago

This is why I need to be able to retrieve x and y from Skyfield, so that I can use them also. I can already get dut1.

I haven't had time to document it yet, but I was able to sneak a new feature into the release early last week:

t.polar_motion_angles()

It returns s´, x, and y as arrays of the same length as the time. Feel free to try it out and let me know if the units and magnitudes seem to make sense.

mkbrewer commented 3 years ago

Since the main sensitivity for observed coordinates Alt/Az and HA/Dec is dut1, I think it would be fairly safe to just ignore polar motion when you have no data and are relying on a model for dut1. Tapering to zero in order to avoid a discontinuity sounds like a good way to handle this to me. .

mkbrewer commented 3 years ago

Ok. I'll update my version of Skyfield and try that out.

mkbrewer commented 3 years ago
ts = load.timescale()
ut = ts.utc(times)
sp, xp, yp = ut.polar_motion_angles()
print len(ut), xp, yp

Results in two scalars, both zero.

1441 0.0 0.0
mkbrewer commented 3 years ago

It'd be nice if these were attributes to be consistent with dut1 so that I could just do this:

ut.dut1[i], ut.xp[i], ut.yp[i]
brandon-rhodes commented 3 years ago

Results in two scalars, both zero.

Oh, we need to install a table first, yes. Official routines and documentation will take me a couple of days of work when I get the opportunity. But here's a preview — it will be slightly off because this sample code interpolates into the table using the wrong JD (by the difference between TT and UTC), but I think the difference wouldn’t be detectable to a radio telescope:

import re
import numpy as np
from skyfield import api

r = re.compile(b'^......(.........) . '
               b'(.\d.......)......... '
               b'(.\d.......).........  '
               b'.(.\d........)', re.M)

with api.load.open('finals2000A.all') as f:
    data = np.fromregex(f, r, [
        ('mjd_utc', np.float32),
        ('x', np.float32),
        ('y', np.float32),
        ('dut1', np.float32),
    ])

jd = data['mjd_utc'] + 2400000.5

ts = api.load.timescale()
ts.polar_motion_table = jd, data['x'], data['y']

t = ts.utc(2020, 11, 22)
sprime, x, y = t.polar_motion_angles()
print(sprime, x, y)

The result:

-9.818851133037312e-06 0.12602078141827727 0.28755390697839467

Your suggestion of a convenience attribute xp and yp is an interesting one. I wonder what names would make the most sense to folks? The IERS seems to call the quantities x and y without the qualifying p, and I have seen other names entirely for them in another reference. The nice thing about polar_motion_angles() is that the name is quite explicit, even for folks not very familiar with them.

mkbrewer commented 3 years ago

I use sp, xp and yp as that is how they are designated in SOFA and USNO Circular 179 (for xp and yp, at least). I suppose you could go with sprime, xpole and ypole also. x and y just seem a bit too generic to me.

As far as the radio telescope is concerned, my program was originally written in order to be able to make moon centered maps to study the sidelobes of our 1.5 degree beam, so none of this is important there. But, being a long time celestial mechanic, I kept on refining it until I got beyond the precision of SOFA just for fun.

mkbrewer commented 3 years ago
IOError: [Errno 2] No such file or directory: './finals2000A.all'

Where is this file supposed to reside? I don't see it in .skyfield nor in dist-packages/skyfield/data.

brandon-rhodes commented 3 years ago

Try downloading it — it turns out my little script was building its timescale with builtin=False, so had already downloaded the file. Here’s an improved approach:

url = api.load.build_url('finals2000A.all')
with api.load.open(url) as f:
    ...
mkbrewer commented 3 years ago

Sorry, I'm not getting anywhere with this.

>>> import re
>>> import numpy as np
>>> from skyfield import api
>>> 
>>> r = re.compile(b'^......(.........) . '
...                b'(.\d.......)......... '
...                b'(.\d.......).........  '
...                b'.(.\d........)', re.M)
>>> 
>>> url = api.load.build_url('finals2000A.all')
>>> with api.load.open('finals2000A.all') as f:
...     data = np.fromregex(f, r, [
...         ('mjd_utc', np.float32),
...         ('x', np.float32),
...         ('y', np.float32),
...         ('dut1', np.float32),
...     ])
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 312, in open
    return open(path, mode)
IOError: [Errno 2] No such file or directory: './finals2000A.all'

I still don't see that file in my current directory (which is where it seems to be looking for it for some reason), nor do I see it in my /home/mkbrewer/.skyfield directory which is where I usually tell Skyfield to load from, nor do I see it in /usr/local/lib/python2.7/dist-packages/skyfield/data. In both of those, there is only deltat.data and deltat.preds.

brandon-rhodes commented 3 years ago

Sorry, I'm not getting anywhere with this.

It’s okay! You just missed a small detail, the change that would have used the url value you computed. If you compare my code to yours, you'll see you are providing open() with the filename instead of switching to telling it what URL to download.

mkbrewer commented 3 years ago

Whoops. I missed one of your instructions there, but it still didn't work:

>>> import re
>>> import numpy as np
>>> from skyfield import api
>>> 
>>> r = re.compile(b'^......(.........) . '
...                b'(.\d.......)......... '
...                b'(.\d.......).........  '
...                b'.(.\d........)', re.M)
>>> 
>>> url = api.load.build_url('finals2000A.all')
>>> with api.load.open(url) as f:
...     data = np.fromregex(f, r, [
...         ('mjd_utc', np.float32),
...         ('x', np.float32),
...         ('y', np.float32),
...         ('dut1', np.float32),
...     ])
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 317, in open
    path = self._assure(url, filename, reload, backup)
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 211, in _assure
    download(url, path, self.verbose, backup=backup)
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 510, in download
    raise e2
IOError: cannot download ftp://ftp.iers.org/products/eop/rapid/standard/finals2000A.all because urlopen() got an unexpected keyword argument 'cafile'
brandon-rhodes commented 3 years ago

Thanks, that's substantial information: it means that the machinery for downloading URLs from secure "https" urls is not being properly invoked in your version of Python. The paths in your exception indicate Python 2.7, which Skyfield still supports, so let me bring up Python 2.7 here on my laptop and see what's going on. I'll report back.

mkbrewer commented 3 years ago

That's quite possible. I saw this:

/usr/local/lib/python2.7/dist-packages/pip/_vendor/urllib3/util/ssl_.py:137: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings
  InsecurePlatformWarning

during the install. I have 2.7.3.

brandon-rhodes commented 3 years ago

Thanks, that indeed looks ominous!

The Python 2 here on my laptop, 2.7.18, works like Python 3 with respect to the SSL support in Skyfield. In your case, Skyfield is falling back to some code in an if statement offered by an AstroPy contributor early this year, I think to get their CI working? In any case, with Python 2.7 support disappearing from so many CI systems, and especially older 2.7's like 2.7.3, I'll have to put something together myself. I'll try "pyenv", plus an integration test that does real downloads (unlike Skyfield's mainline tests), plus virtual environments, plus a bit of bailing wire.

mkbrewer commented 3 years ago

This gets more and more interesting. I just went ahead and downloaded it myself and put it in my .skyfield directory. Now I get:

>>> import re
>>> import numpy as np
>>> from skyfield.api import Loader
>>> 
>>> LOAD_FROM = '/home/mkbrewer/.skyfield'
>>> load = Loader(LOAD_FROM)
>>> 
>>> r = re.compile(b'^......(.........) . '
...                b'(.\d.......)......... '
...                b'(.\d.......).........  '
...                b'.(.\d........)', re.M)
>>> 
>>> with load.open('finals2000A.all') as f:
...     data = np.fromregex(f, r, [
...         ('mjd_utc', np.float32),
...         ('x', np.float32),
...         ('y', np.float32),
...         ('dut1', np.float32),
...     ])
... 
>>> jd = data['mjd_utc'] + 2400000.5
>>> 
>>> ts = load.timescale()
>>> ts.polar_motion_table = jd, data['x'], data['y']
>>> 
>>> t = ts.utc(2020, 11, 22)
>>> sprime, x, y = t.polar_motion_angles()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/skyfield/timelib.py", line 745, in polar_motion_angles
    return sprime, interp(self.tt, tt, x), interp(self.tt, tt, y)
  File "/usr/local/lib/python2.7/dist-packages/numpy/lib/function_base.py", line 2057, in interp
    return interp_func([x], xp, fp, left, right).item()
ValueError: array of sample points is empty
>>> print(sprime, x, y)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'sprime' is not defined
>>> 
mkbrewer commented 3 years ago

I notice that final2000A.all has a y2k problem. finals2000A.all.csv is better in that regard and it even has column headers.

brandon-rhodes commented 3 years ago

I just went ahead and downloaded it myself…

Thanks for moving us along while I in the meantime have been facing the problem of pyenv's not being able to install Python 2.7.3 on my system. Support for SSL in 2.7.3 might be a big project to tackle this week, but we'll see!

has a y2k problem

Fortunately its MJD column allows the calendar date fields to be ignored.

ValueError: array of sample points is empty

Alas, I would need to examine each of the arguments to interp() to be sure of what's going on. It sounds like it's unhappy with the arrays that are members of the polar motion table.

mkbrewer commented 3 years ago

Does it really need SSL? It's just a plain old ftp site. I just tried it on my old Windows XP laptop using IE8 which doesn't have a clue regarding SSL and it worked fine.

brandon-rhodes commented 3 years ago

Does it really need SSL?

Alas, Skyfield always has to pass the correct arguments to urlopen() to be ready for SSL. While one might expect that it could try introspecting the URL itself for the presence of the "s" indicating SSL, I have avoided that both because of the additional complexity, and also because of the case where a non-SSL HTTP request might 301-redirect to the same URL but with HTTPS enabled.

I suppose I could special-case ftp: since it can't do redirection, but that would just be a temporary stopgap; I still need to support passing the correct urlopen() arguments in Python 2.7.3 if I'm going to fully support it. I've just landed (see the link just above your previous comment) a more robust detection mechanism than the one contributed early this year: it directly checks whether it can call urlopen() with a cafile=None argument instead of doing any guessing based on Python versions. So hopefully it will even work in cases where an operating system backported the argument to a version that didn't originally support it.

mkbrewer commented 3 years ago

btw, here's why I got that error:

>>> ts.polar_motion_table
(array([], dtype=float32), array([], dtype=float32), array([], dtype=float32))

The table is empty.

mkbrewer commented 3 years ago

This is odd because Skyfield 1.10 had no problem downloading de430_1850-2150.bsp.

mkbrewer commented 3 years ago

So this must be new. I just tried downloading de432t.bsp and got the same issue.

    planets = load('de432t.bsp')
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 190, in __call__
    path = self._assure(url, filename, reload, backup)
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 211, in _assure
    download(url, path, self.verbose, backup=backup)
  File "/usr/local/lib/python2.7/dist-packages/skyfield/iokit.py", line 510, in download
    raise e2
IOError: cannot download ftp://ssd.jpl.nasa.gov/pub/eph/planets/bsp/de432t.bsp because urlopen() got an unexpected keyword argument 'cafile'
brandon-rhodes commented 3 years ago

The table is empty.

If the file was downloaded successfully and is non-empty, then we will have to figure out whether the problem is (a) that the RE doesn't properly match the file's contents in Python 2.7.3 like it does in Python 2.7.18 (which I just double-checked against), or (b) whether the fromregex() routine behaves differently in your version of NumPy than in more recent versions.

Maybe try:

with load.open('finals2000A.all') as f:
    raw_bytes = f.read()
    print(r.findall(raw_bytes))

— just after defining the RE and see if a huge screenful of matches print out?

This is odd because Skyfield 1.10 had no problem…

Skyfield 1.10 is from February 2009, more a year before the AstroPy contribution that tried to fix a behavior they were seeing under Python 3.6, so I expect it won't show the problems that I suspect were inherent in the approach they used.