das-developers / das2java

The original das2 library. Provides interactive publication-ready 2-D plotting
https://das2.org
GNU Lesser General Public License v3.0
4 stars 0 forks source link

TimeUtil.toTimeArray and UnitsConverter unstable, handling of leap seconds #44

Open jbfaden opened 1 year ago

jbfaden commented 1 year ago

A noisy test of Autoplot (https://cottagesystems.com/jenkins/job/autoplot-test005/18804/, 5th bookmark) showed where there's an instability in the Das2 code. The Units class builds up a table of converters, and then these converters are automatically connected such that new conversions work automatically. (cm→m and cm→ft can derive m→ft.) This has a problem though that the order that units things are added matters, as different paths can be derived. Logging would trigger a different path and the bug would show in test005. Change the log levels or logger format, and the bug would go away.

Note too that though there was a direct route from cdfEpoch to "hr since 2001-01-01T00:00:00.000Z" the conversion from ms to hr might not be. (Something doesn't jibe there, since ms to hr should be quite stable, so it might be a different offset conversion, but this should be considered as well.)

To resolve this I'm going to take out the conversion part of the code which decomposes times and make sure this is stable.

jbfaden commented 1 year ago

Also when looking at this, I realize to redo TimeUtil.java's toTimeArray (and fromDatum), I really need to remove how the leap seconds table is loaded. Having an external dependence and file load, often on the event thread, is not acceptable. Also there hasn't been a leap second since 2017, and there's lots of talk of getting rid of them. I would rather have a stable and predictable code which needs to be updated when there's a leap second, than to have an external dependence for the library.

jbfaden commented 1 year ago

There's a lot of chatter in this logger which I don't understand, making it look like it's registering conversions when it shouldn't be. Note once a conversion is derived, it should be registered and used again. Further, there are optimizations which could be made but might not be, such as [scale conversion] append [scale conversion] should result in one [scale conversion], but it might chain them together.

This hasn't been reviewed in years (credit to Ed, where it's just worked nicely), 15+ probably, and probably should be.

Also there's an SI units class, which supports multiply and divide operations, which might be brought in as well.

jbfaden commented 1 year ago

See also https://cdf.gsfc.nasa.gov/html/CDFLeapSeconds.txt

jbfaden commented 1 year ago

Another manifestation of this is how units are picked. Sometimes in testing Autoplot "10 msec" is printed instead of "10ms". This should also be deterministic.