shashwatak / satellite-js

Modular set of functions for SGP4 and SDP4 propagation of TLEs.
MIT License
902 stars 145 forks source link

jday() and invjday() do NOT keep milliseconds #75

Open abolfazlshirazi opened 4 years ago

abolfazlshirazi commented 4 years ago

I realized that two functions jday() and invjday() skip computations for milliseconds. In jdayInternal(), it is:

function jdayInternal(year, mon, day, hr, minute, sec, msec = 0) {
...
}

where msec = 0 as the input sets milliseconds to zero. Also, in invjday(), it is:

export function invjday(jd, asArray) {
...
  return new Date(Date.UTC(year, mon - 1, day, hr, minute, Math.floor(sec)));
}

where Math.floor(sec) as the output eliminates milliseconds.

It is easy to test by converting and reconverting:

var mytime = new Date();
console.log(mytime.getMilliseconds()); // it returns some milliseconds.
var newtime = satellite.invjday( satellite.jday(mytime) );
console.log(newtime.getMilliseconds()); // it returns zero!

These functions need revisions.

thkruz commented 3 years ago

I am looking changing the logic in days2mdhms to

let temp = (days - dayofyr) * 24.0;
const hr = Math.floor(temp);
temp = (temp - hr) * 60.0;
const minute = Math.floor(temp);
temp = (temp - minute) * 60.0;
const sec = Math.floor(temp);
const msec = (temp - sec) * 1000.0;

Currently I am within +/- 1 millisecond. I will fiddle with it more tomorrow to see if I can figure out the cause. Not sure if the OP can say if it matters for his particular use. Either way this will be better than it was before.

thkruz commented 3 years ago

As expected - this is breaking 18 tests because of the tolerance in the tests and a change of up to 999 milliseconds being introduced.

Any thoughts on using alternative verification methods? There are some high accuracy laser satellites (I'd have to look at whether or not the data is available for this forum). If this millisecond issue is in the actual SGP4 code vs the JS port then it is quite an interesting find...

davidcalhoun commented 3 years ago

Just for context, I believe I first added milliseconds to jdayInternal to the library originally, so it wasn't in the original JS port.

In the tests we could possibly use toBeCloseTo(), not sure what folks would think of that.

ezze commented 3 years ago

@davidcalhoun Here is your PR for milliseconds support.

My first thought is the same: take a look at toBeCloseTo. The second one is to check original Python library regarding milliseconds support. All the work on porting the library was made by @shashwatak many years ago but I don't believe he is available now. Chances that satellite.js is outdated are very high.