Tronald / CoordinateSharp

A library designed to ease geographic coordinate format conversions, and determine sun/moon information in C#
Other
364 stars 59 forks source link

Local Time Conversion Efficiency #115

Closed Tronald closed 5 years ago

Tronald commented 5 years ago

Current local time conversions for celestial calculations are 3 times as expensive as UTC conversion. This is due to how the library's strict UTC operation works.

In reviewing the trigonometry, we may be able to simply provide a UTC offset julian value in the julian conversion logic. This would eliminate all additional overhead during local time conversions.

Offset could be added or subtracted from Jrise at this line for example:

https://github.com/Tronald/CoordinateSharp/blob/f439177f68032a753e2cbae441ba938942b9ac10/CoordinateSharp/Celestial/Solar/SunCalculations.cs#L157

Each hour is approximately .04166667 in Julian.

  1. Logic will need to be added to handle passing UTC offset.
  2. Logic will need to be added to make date null if event date is not the same as the passed date.

During research it has been determined that local time conversion in conjunction with UTC calculations are too expensive. Local time conversions will remain Lazy.

Additionally, the following method to receive local time should be added.

Celestial celestial = coordinate.Celestial.ToLocalTime(offset);

This issue will serve as a tracker for trigonometry adjustments to account for local time conversions until complete.

Tronald commented 5 years ago

UTC Offset variable has been added to Sun Angle calculations as well as Solar Event Date calculations in testing. Benchmarks for solar local times match UTC benchmarks. Once Lunar conversions are complete extensive tests will be added for local time to ensure accuracy.

These changes are minimal and should not have any negative effect on the celestial trig, while greatly improving benchmarks.

Tronald commented 5 years ago

UTC Offset adjustments have been added to Lunar Events in testing. Dynamic Time must remain in UTC during calculations however, but this had 0 ill effect. Lunar local time benchmarks now match UTC benchmarks.

Because of this fact, an option is being considered to add an Offset property to the Coordinate object to allow the user to operate in either UTC of Local. This will slightly delay the next release due to testing requirements, but may be worth it as this would be a huge improvement to the library.

Unfortunately, the library will not be able to utilize DateTimeKind of the GeoDate property to determine offset due to structuring and the ill effect it would have on existing applications, but a simple offset property would be very minimal code on the users end and would allow both Eager and Lazy loaded local times.

Tronald commented 5 years ago

Current Solar / Lunar "only" methods will be marked obsolete in favor of an instance Coordinate.Celestial_LocalTime(double offset) function and a static Celestial.CalculateCelestialTimes(double lat, double longi, DateTime date, EagerLoad el, double offset) function. Instead of needing multiple functions to handle different celestial loads, 2 functions can handle all scenarios with the newly added eager loading extensions from 2.2.2.1.

Benchmarks for Solar/Lunar cycle only Local time conversions are now hitting below 1 ms when proper library performance enhancements are applied.. Though the current local time methods are newer, the performance gain and simplicity improvement is to good to ignore. The depracated methods have also been improved for the next release, but will receive no new updates after thereafter.

Tronald commented 5 years ago

In testing is has been discovered that improving upon the methods scheduled to be deprecated one last time is potentially breaking. These method will need to be reverted back to the original, slower calculations.

Because of this, developers who do not convert to the newer methods will see no performance gain. Implementation of the change is extremely easy however, and examples will be provided in the release info.

Tronald commented 5 years ago

New methods have been implemented, old methods have been deprecated and testing/documentation has been updated. Scheduled for next release.

-Local time conversions are now efficient. -Coordinate may operate in Local instead of UTC if specified. -Local time conversions will now make use of EagerLoading_Extensions