sgp4 is no longer no-arch

[WeatherGod]

It now has a useful compiled component

Checklist

• [x] Used a fork of the feedstock to propose changes
• [x] Bumped the build number (if the version is unchanged)
• [ ] Reset the build number to 0 (if the version changed)
• [ ] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
• [ ] Ensured the license file is being packaged.

[conda-forge-linter]

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

[WeatherGod]

closes #18

[WeatherGod]

Perhaps it would make sense to have some sort of build test to ensure that the accelerated stuff got built? Not exactly sure how that should get implemented, though.

[jochym]

@conda-forge-admin, please rerender

[jochym]

@brandon-rhodes it looks like we have problem with OSX build. The test_all_three_gravity_models_with_sgp4init and test_all_three_gravity_models_with_twoline2rv fail with 16th significant digit difference. Is this reasonable? 1e-16 relative difference seems small. Can you check this?

[jochym]

@WeatherGod thanks for spotting the problem with 2.0 packages. To change the build matrix you need to rerender the recipe to update CI scripts.

[jochym]

@brandon-rhodes I have noticed that sgp4 uses assertAlmostEqual which tests number of decimal places not significant digits. Shouldn't we rather use significant digits and relative differences?

[WeatherGod]

the pipelines look like it is building just for py3.6/3.7, but not 3.8. Don't know there is something else that is needed to be done, or if 3.8 packages aren't "popular" yet?

[WeatherGod]

What is probably happening with the test failures is that since it is now going through the accelerated pathway, it is coming out slightly different than before. However, I am not an expert in interpreting SGP4 failures, so I don't know if differences in the 16th significant digit is expected or not.

[jochym]

@WeatherGod I think the py3.8 builds are not run yet.

[WeatherGod]

Do you mean that those 3.8 runs are still pending on this commit, or is it that the conda-forge-admin bot doesn't do py3.8 builds yet? Because the configs do not have 3.8 entries at all. Just a little surprised to not see py3.8, but that is probably a discussion for elsewhere.

Do you think it is going to be a problem that build 0 will be effectively available for py3.* while build 1 will only be available for py3.6/7?

[jochym]

@WeatherGod I will remove the previous build when we have the new one. I think the c-f is not running 3.8 builds in stable branch yet.

[brandon-rhodes]

@brandon-rhodes it looks like we have problem with OSX build. The test_all_three_gravity_models_with_sgp4init and test_all_three_gravity_models_with_twoline2rv fail with 16th significant digit difference. Is this reasonable? 1e-16 relative difference seems small. Can you check this?

Given two machines running the same C code, I had previously been able to expect results that match to machine precision, which for 64-bit floats is 16 decimal places.

Where are the logs showing the failure? If we retreat to 15 digits will that solve things, or are the results farther off than that?

[jochym]

@brandon-rhodes Just look at the recent build details in this PR (e.g. OSX build for py37). Here is a quote:

======================================================================
FAIL: test_all_three_gravity_models_with_sgp4init (sgp4.tests.TestFunctions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/miniforge3/conda-bld/sgp4_1592940565631/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold/lib/python3.7/site-packages/sgp4/tests.py", line 228, in test_all_three_gravity_models_with_sgp4init
    assert_wgs84(sat)
  File "/Users/runner/miniforge3/conda-bld/sgp4_1592940565631/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold/lib/python3.7/site-packages/sgp4/tests.py", line 246, in assert_wgs84
    assertAlmostEqual(r[0], -3754.2437675772426, GRAVITY_DIGITS)
AssertionError: -3754.2437675772417 != -3754.2437675772426 within 12 places (9.094947017729282e-13 difference)

======================================================================
FAIL: test_all_three_gravity_models_with_twoline2rv (sgp4.tests.TestFunctions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/miniforge3/conda-bld/sgp4_1592940565631/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold/lib/python3.7/site-packages/sgp4/tests.py", line 208, in test_all_three_gravity_models_with_twoline2rv
    assert_wgs84(Satrec.twoline2rv(LINE1, LINE2, WGS84))
  File "/Users/runner/miniforge3/conda-bld/sgp4_1592940565631/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold/lib/python3.7/site-packages/sgp4/tests.py", line 246, in assert_wgs84
    assertAlmostEqual(r[0], -3754.2437675772426, GRAVITY_DIGITS)
AssertionError: -3754.2437675772417 != -3754.2437675772426 within 12 places (9.094947017729282e-13 difference)

----------------------------------------------------------------------

The difference is quite small, but it depends on the algorithm about which I know nothing. Regardless, I still think this should be N significant digits (i.e. relative difference) and not digits after the decimal point. I agree that it would be better to have the same results but sometimes it is simply impossible on different platform, and we have to settle for consistent.

[brandon-rhodes]

I still think this should be N significant digits (i.e. relative difference) …

The problem with relative difference, I have found across several libraries, is that it discards the physical meaning of the measurement. Imagine a test that asks for the x-coordinate of a satellite in orbit around the Earth at, say, 8500 km. Depending on whether the satellite happens to be sitting far out along the x-axis or instead is nearly perpendicular to it, its x coordinate might be around 8500 km, or 8.5 km, or even 0.0085 km.

A demand that all three of these values are accurate to, say, 4 digits of precision would demand precisions (if I'm counting digits correctly?) respectively of 1km, of 1m, and of 1mm.

The problem is that algorithms producing physical coordinates tend to be accurate and reproducible to within some fixed physical distance. They do not automatically get more precise just because a coordinate they produce happens to be a small value relative to the origin. If, say, the SGP4 algorithm is good to within 1m, then tests need to compare to within that physical distance. Just because the x-coordinate gets very close to the origin does not mean that it suddenly becomes much more precise.

If that behavior I believe I've seen is in fact a property of the algorithm (and I'm open to an analysis of the actual error noise showing that my impression is wrong), then the fixed error bars I am setting is the correct approach. But I really should be expressing them physically, rather than in abstract numeric terms, which I'll see about correcting as I go in to reduce the number down to 11 places. We'll see if that fixes the tests. (I wonder if there's an easy way for me to run my test suite on macos, and not simply the build/release routine on macos?)

[brandon-rhodes]

@jochym — Well, drat. I just built the library on an old MacBook that I happened to have handy, and it’s passing its tests just fine. In particular test_all_three_gravity_models_with_sgp4init runs without any error. So I will not have any way of verifying whether a fix I make really resolves the problem, absent your attempting a build over on your side.

Will a new release be necessary simply to update that one test, hoping that it fixes the problem on your end? If so then I hope we get it right on the first release!

[jochym]

@brandon-rhodes I agree about precision. I was wrong about relative differences. But you are also right about physical error bars instead of just number of decimal places. Ultimately these algorithms are precise within physical distances - regardless of the coordinate system and/or units.

Regarding the build. I think this is connected with particular toolchain used for conda-forge build. It may be hardware or software issue. I have no idea what hardware Microsoft is using for azure build machines for OSX. As for testing the solution: if you can provide a patch to apply to the source before building I can put it in for testing and if it works we can do it properly with a new release.

[brandon-rhodes]

As for testing the solution: if you can provide a patch to apply to the source before building I can put it in for testing and if it works we can do it properly with a new release.

Wonderful! Thank you for coming up with that option. Here is a patch (is this a good format? let me know and I can produce it in another format if you need it) that, when tested with the numbers you displayed above, should let the tests pass on a non-Linux system:

diff --git sgp4/tests.py sgp4/tests.py
index 9b65c16..b03b30e 100644
--- sgp4/tests.py
+++ sgp4/tests.py
@@ -6,6 +6,7 @@ except:
     from unittest import TestCase, main

 import datetime as dt
+import platform
 import re
 import sys
 from doctest import DocTestSuite, ELLIPSIS
@@ -232,7 +233,14 @@ def test_all_three_gravity_models_with_sgp4init():
     sat.sgp4init(WGS84, 'i', VANGUARD_ATTRS['satnum'], VANGUARD_EPOCH, *args)
     assert_wgs84(sat)

-GRAVITY_DIGITS = 12 if accelerated else 4
+GRAVITY_DIGITS = (
+    # Why don't Python and C agree more closely?
+    4 if not accelerated
+    # See https://github.com/conda-forge/sgp4-feedstock/pull/19 for details.
+    else 11 if platform.system() != 'Linux'
+    # Insist on very high precision on my Linux laptop.
+    else 12
+)

 def assert_wgs72old(sat):
     e, r, v = sat.sgp4_tsince(309.67110720001529)

[jochym]

@brandon-rhodes it worked. My proposition is to merge this to fix noarch issue and ask you to release new version, maybe after considering why this is happening on OSX and Windows. I know that Linux is better ;) and the others are toys not suitable for serious work ;) - but such problems make me slightly uncomfortable. I will wait with merging for your OK. @WeatherGod : are you OK with this plan?