Add integration of TT-TDB; add and update GHAs

[PerezHz]

This PR adds to the DE430! dynamical model the integration of TT-TDB following essentially the approach of Fienga et al. (2009), while adding the contribution to TT-TDB due to the Sun's J_2 as is done in the DE430/431 integration.

cc: @lbenet @LuEdRaMo

[PerezHz]

P.S. I can rebase this PR after merging #16, if the latter is ready to be merged

[PerezHz]

The core of the changes of this PR is at the end of src/dynamical_model.jl, with corresponding changes in src/jetcoeffs.jl; the rest of the changes are either aesthetic or minor updates.

[PerezHz]

(rebased to current main)

[PerezHz]

Since it's a small and independent change, I'm also adding here CI and CompatHelper for PlanetaryEphemeris just to avoid having to open another PR; otherwise this is ready for review

[PerezHz]

Update: tests are showing that datetime2julian tests for Float128 are failing since, AFAIU, the corresponding methods were removed in #16 to avoid type piracies. I temporarily commented out those tests to see how CI reacts, but since the only allowed initial conditions are the ones in 1969 and 2000, I would think that those tests are maybe not longer needed? How do you suggest @lbenet @LuEdRaMo we should handle this?

[codecov-commenter]

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@1188f11). Click here to learn what that means. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #17   +/-   ##
=======================================
  Coverage        ?   42.62%           
=======================================
  Files           ?       12           
  Lines           ?    16586           
  Branches        ?        0           
=======================================
  Hits            ?     7070           
  Misses          ?     9516           
  Partials        ?        0           

[lbenet]

Update: tests are showing that datetime2julian tests for Float128 are failing since, AFAIU, the corresponding methods were removed in #16 to avoid type piracies. I temporarily commented out those tests to see how CI reacts, but since the only allowed initial conditions are the ones in 1969 and 2000, I would think that those tests are maybe not longer needed? How do you suggest @lbenet @LuEdRaMo we should handle this?

For the time being, I think it is ok to have them commented. If we want those type of checks, I think a PR should be opened in Dates...

[LuEdRaMo]

I think I have some better tests and a few improvements to the selecteph function. Should I push those changes here or leave it for another PR?

[PerezHz]

I think I have some better tests and a few improvements to the selecteph function. Should I push those changes here or leave it for another PR?

It will take me a couple of days to address all of @lbenet's review points (hopefully it'll be ready by tomorrow!), so if your tests are already on top of current main branch, maybe it makes sense to open another PR; then we can rebase here if needed

[LuEdRaMo]

It will take me a couple of days to address all of @lbenet's review points (hopefully it'll be ready by tomorrow!), so if your tests are already on top of current main branch, maybe it makes sense to open another PR; then we can rebase here if needed

Actually the tests are on the top of this PR.

[lbenet]

Just a trivial observation from the last tests run: v1.6 takes 31m to complete the tests, while v1.9 19m, and nightly ~10!

[LuEdRaMo]

Just as a reference, after 100 steps (roughly 228.5 days), the difference in TT-TDB between PE and JPL is approximately 1.56e-10 seconds.

[PerezHz]

Actually the tests are on the top of this PR.

Alright, then it's just easier to push here; feel free to push to this PR as soon as you're ready 😄

[PerezHz]

Just as a reference, after 100 steps (roughly 228.5 days), the difference in TT-TDB between PE and JPL is approximately 1.56e-10 seconds.

0.1 ns in ~1 yr is not too bad... Did your run include all the asteroids?

[LuEdRaMo]

0.1 ns in ~1 yr is not too bad... Did your run include all the asteroids?

Yes. I will do a 100 years integration to see the long-term behaviour, but that will take some time.

[LuEdRaMo]

Tests seem to work but they are slow. Is there a way you know to run them on multiple threads?

[lbenet]

I think there is a way to use more threads in the tests, but I'm not so sure if that would be the best with respect to Github...

[lbenet]

Tests seem to work but they are slow.

Could it be the precompilation step?

[PerezHz]

Tests seem to work but they are slow. Is there a way you know to run them on multiple threads?

There is indeed a way, by passing the JULIA_NUM_THREADS environment variable to the test run step in the CI worfklow, which I've now added to CI.yml file (I think there is a limit to use only two threads). I actually think it makes sense to perform our tests using two threads, if not for the sake of performance itself, to actually test the propagation in a multi-threaded environment.

[PerezHz]

Could it be the precompilation step?

I think it is

[PerezHz]

Could it be the precompilation step?

I think the longer times are (maybe?) also due to the fact that the tests also propagate using the Float128 methods, so we are in a sense precompiling the specialized jetcoeffs! & co. methods twice (one for Float64, one for Float128)

[lbenet]

I think the longer times are (maybe?) also due to the fact that the tests also propagate using the Float128 methods, so we are in a sense precompiling the specialized jetcoeffs! & co. methods twice (one for Float64, one for Float128)

I think we should only compile Float64, which is what we currently use mostly. Also, percompiling few methods in TaylorSeries and TaylorIntegration should make precopilation easier here.

[PerezHz]

I think we should only compile Float64, which is what we currently use mostly. Also, percompiling few methods in TaylorSeries and TaylorIntegration should make precopilation easier here.

I agree with that; I pushed a commit to see how precompilation goes with Float128, but maybe we can settle for the time being on precompilation and tests only for Float64? And I also agree that precompiling stuff on TS and TI will help us here

[lbenet]

Tests pass now, except in Julia v1.6, because the tests were cancelled; I suggest not to test that version of Julia.

[PerezHz]

Tests pass now, except in Julia v1.6, because the tests were cancelled; I suggest not to test that version of Julia.

Indeed, there seems to be a 2 hour timeout for CI workflows which the testing on julia1.6 exceeds; I modified CI.yml to test only 1.9 and nightly for now. This still leaves open the question of what to do with JuliaRegistrator: there is a 1 hour timeout for autoMerge tasks when registering a package in the General registry, and since loading the package for the first time (i.e., doing import PlanetaryEphemeris) with precompilation enabled takes currently more than an hour there, we haven't been able to automatically register PlanetaryEphemeris v0.5.1 and something similar might happen with newer releases as well: one of the requirements for autoMerge is that packages should be loadable, but if loading takes more than an hour then the job is not finished and autoMerge fails. I actually think we maybe should comment out precompile.jl from the includes until we improve TTFL for PlanetaryEphemeris, what do you think?

[lbenet]

I was thinking the same: simply comment the precompilation step, and see how that behaves...

[LuEdRaMo]

I added a commit to try commenting out the precompilation file and doing only the Float64 tests.

[PerezHz]

I was thinking the same: simply comment the precompilation step, and see how that behaves...

Agreed, and thanks @LuEdRaMo for pushing the corresponding changes!

As a side note, maybe we can consider somehow taking advantage of PackageCompiler.jl to tackle the issues here with TTFL. I'm thinking out loud here, but maybe we could build a system image containing the compiled jetcoeffs! & co. methods defined by PlanetaryEphemeris, and host that sysimg it somewhere and then just retrieve it when needed... For me it makes sense, since the dynamical model is not so frequently updated, so we would only re-build it every now and then.

Another option can be to put the dynamical model methods as a package extension, and load it as a weakdep, thus it doesn't get in the way of loading/compiling times. Of course, this latter option only delays/postpones the precompilation of the jetcoeffs! method until it's used... but no need to decide anything now; just putting some ideas around 😄

[PerezHz]

I just pushed a commit fixing a small issue with the tests, and updated the test env setup via a test/Project.toml

[PerezHz]

... and it went down to 32 minutes on 1.9 and nightly! I think we're in business, folks!

[PerezHz]

I'm currently adding some comparisons with the JPL ephemeris in the tests; hope to have that ready ideally by today...

[lbenet]

This this is almost ready, modulo the tests you are adding.

The question is how shall we proceed now? Merge this and tag the corresponding version as 0.6.0 (and simply forget about 0.5.1)?

[PerezHz]

The question is how shall we proceed now? Merge this and tag the corresponding version as 0.6.0 (and simply forget about 0.5.1)?

I agree going with v0.6.0; just updated the Project.toml accordingly. If tests pass then this PR will be ready to be merged.

[PerezHz]

Tests are passing so I'll go ahead and merge.