Update JulianDate calculations

[cusher]

This is meant to address a handful of issues in the JulianDate class:

1. Converting certain JD values to a LocalDateTime would throw an exception, e.g.

   JulianDate.of(2457754.8).toLocalDateTime()

   Invalid value for NanoOfSecond (valid values 0 - 999999999): 1000000000

2. The fact that milliseconds are generally truncated rather than rounded leads to some strange conversions results, e.g.

   JulianDate.of(2457754.4).toMilliseconds()

   1483220159999

   Notably toMilliseconds would sometimes return a different time than toLocalDateTime.

3. Converting from MJD/TJD/RJD is more complicated than going the other direction.

4. Adding or subtracting large double values throws an error (not sure if this is intentional for optimization purposes, but if so it was undocumented).

5. No ability to take in/return an Instant.

One thing that is hinted at above but not addressed, is the fact that even though one might expect conversion to/from milliseconds and LocalDateTime (and/or Instant) to always result in the same value, the fact different calculations are used in the two code paths makes me suspect there might still be a way to get different results from the two (even though with the rounding changes the cases I tried seem to be fixed). Thus I would also suggest it might be having a single representation used as the base for all such conversions. If such a change is welcome, I can add this as well, but it would be good to know which method is considered more accurate/precise.

If there are any changes needed here, I'm happy to make another pass at this.

Thanks!

[igr]

Thank you, @cusher; I loved the changes! You rock 🚀 ❤️

Some notes:

• I have changed Math.round() to Math.floor() to maintain the same precision (0.001 seconds) - otherwise, the tests would fail. I remember that I had to do so to make tests work - all the numbers in the tests were taken from some 3rd party site (I can't remember from where).
• Thank you for Instant support - this library was made long before Java 8, and this was needed!

Thus I would also suggest it might be having a single representation used as the base for all such conversions. If such a change is welcome, I can add this as well, but it would be good to know which method is considered more accurate/precise.

Are you talking about methods of() and toLocalDateTime()? These two should be symmetrical... I know I tested a lot of conversions to/from java dates; but sure - let's discuss; I am all open to the unification of the code and simplifying the calculations!

🎉 If you need these changes, I can release 6.1.1 immediately, and then we can discuss the modifications and improvements later. Please let me know!

[igr]

Just an update, I put round() back, you were right!

[cusher]

Thanks very much for the quick feedback and merge @igr !

Are you talking about methods of() and toLocalDateTime()? These two should be symmetrical... I know I tested a lot of conversions to/from java dates; but sure - let's discuss; I am all open to the unification of the code and simplifying the calculations

Well, the big picture is that any of LocalDateTime, Instant, or milliseconds should be exactly equivalent (assuming millisecond granularity, and ignoring timezone stuff). So I would expect any conversion to/from any of these with an equivalent value to always result in the same thing.

However, at least prior to making the rounding changes, there are definitive situations where that was not the case, for instance:

jdt = JulianDate.of(2457754.4);
assertEquals(Instant.ofEpochMilli(jdt.toMilliseconds()), jdt.toLocalDateTime().toInstant(ZoneOffset.UTC));
// Test failure

jdt = JulianDate.of(2457754.8);
assertEquals(Instant.ofEpochMilli(jdt.toMilliseconds()), jdt.toLocalDateTime().toInstant(ZoneOffset.UTC));
// Test failure

This raises a series of questions in my mind:

• Which method provides the more accurate answer?
• If there is a definitive answer to the above question, should the less accurate method just call the more accurate method and convert from that?
• If there is not a definitive answer, how does a user of the class decide which one to call?
• Is it also possible that calling of(long) will result in a different value compared with of(LocalDateTime), since there are two different calculations used in each of those?

My first instinct in response to that is that there should be just one calculation method to convert from the internal JulianDate date-time representation to an "Instant-like" date-time representation, and just one calculation method to convert the other way.

Hopefully that clarifies things, let me know if you still have questions.

...All that being said, it may be worth confirming with specific test cases that there are still cases (post rounding changes) where results can differ in unexpected ways.

If you need these changes, I can release 6.1.1 immediately

The changes aren't needed immediately, but that Invalid value for NanoOfSecond error is a bit concerning, so depending on the time-scale of any further changes it might be useful.

[igr]

Got it. Since the code was built before the Instant and other classes, the number of milliseconds was (or should be) used for all calculations and conversions to Java types.

So, the idea was to have a single method with long milliseconds (the most precise one), and other methods should be used that one to calculate JD. The same should happen to convert back.

Let me check the code... I want to fix it.