Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
abolfazlshirazi opened this issue Sep 14, 2020 · 4 comments
Open

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

abolfazlshirazi opened this issue Sep 14, 2020 · 4 comments
Labels

Comments

@abolfazlshirazi
Copy link

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
Copy link
Collaborator

thkruz commented Jan 7, 2021

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
Copy link
Collaborator

thkruz commented Jan 7, 2021

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
Copy link
Contributor

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
Copy link
Collaborator

ezze commented Jan 7, 2021

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants