Closed weatherlym15 closed 7 years ago
Dear @shashwatak,
unfortunately, I have to confirm the trend as described by @weatherlym15.
Indeed, for time instances close to the TLE epoch, the results provided by the js version match to the corresponding ones by the c++ version. Note that my js results are in agreement with those provided for benchmarking purposes, but the time instances there are quite close to the TLE epoch, and hence, the problem is not revealed. However, for time differences wrt TLE epoch larger than a few months, it becomes evident that the results between the two versions are starting to deviate.
Below, as an example you may find results provided by both versions for the Astra 2F (38778) satellite using the following TLE:
1 38778U 12051A 12288.95265372 .00000136 00000-0 00000+0 0 217
2 38778 000.0698 254.6769 0000479 231.1384 284.5280 01.00269150 226
By comparing the two versions, it becomes clear that the results are in agreement only for the TLE epoch time instance. Note that I have verified the validity of my c++ results by equivalent results using the STK software for the TEMEofEpoch system of coordinates.
After enough trials and comparisons, I have concluded that perhaps either the js version has a bug, or progressively loose numerical precision, or the Cartesian position/velocity results versus time since TLE epoch are not in the True Equator, Mean Equinox (TEME) coordinate system, as the results provided by the c++ are.
Please help me to resolve this matter, since your js version is quite useful to all of us.
Thanks for your time, Nikos
Results Obtained for Astra 2F (38778) Based on C++ Version
Time (min) x (km) y (km) z (km) Vx (km/s) Vy (km/s) Vz (km/s) Y M D Time
0.00000000 26932.01821553 32442.36380581 18.67203987 -2.365855637 1.963833427 -0.004652773
500000.00000000 -41642.98362540 6521.85388710 -484.73552503 -0.475966361 -3.038100957 0.005837330 2013 9 27 4:11:49.281394
1000000.00000000 -41679.89221227 -6406.44765047 -888.19222905 0.466036434 -3.038415548 0.041394568 2014 9 9 9:31:49.281416
1500000.00000000 89.92166006 -42138.48887821 783.16731078 3.073618177 0.008372197 0.102829394 2015 8 22 14:51:49.281398
2000000.00000000 40249.05825173 -12456.01761882 2073.91657116 0.910791707 2.935583446 -0.045177132 2016 8 3 20:11:49.281421
2500000.00000000 41670.98646874 6091.87422335 1846.43614861 -0.438088814 3.040224736 -0.144569657 2017 7 17 1:31:49.281403
3000000.00000000 -24943.38429627 33846.85571434 -3202.53618091 -2.473012005 -1.826475386 -0.041201988 2018 6 29 6:51:49.281425
3500000.00000000 -25743.03216099 33183.50696319 -3784.96326117 -2.428125057 -1.885980854 -0.019143710 2019 6 11 12:11:49.281407
4000000.00000000 -19165.85184790 -37493.19360463 1884.67124427 2.732242938 -1.382445535 0.284603946 2020 5 23 17:31:49.281430
Results Obtained for Astra 2F (38778) Based on Javascript Version
Time since epoch: 0 min
Position (km): (x = 26932.01821553236, y = 32442.363805806563, z = 18.672039872509902)
Velocity (km/s): (Vx = -2.3658556373925705, Vy = 1.9638334268968463, Vz = -0.004652773306378888)
Time since epoch: 500000 min
Position (km): (x = -13790.23079846215, y = 39843.11842649649, z = -298.26415348078467)
Velocity (km/s): (Vx = -2.9054892440023568, Vy = -1.0058968645002497, Vz = -0.02848165004581589)
Time since epoch: 1000000 min
Position (km): (x = -41207.47684407635, y = 8867.498679969154, z = -1031.9048243201808)
Velocity (km/s): (Vx = -0.6472942239720777, Vy = -3.005855532153245, Vz = 0.015608178622012314)
Time since epoch: 1500000 min
Position (km): (x = -28986.83394359393, y = -30618.058288413285, z = -404.3298355701939)
Velocity (km/s): (Vx = 2.2304434637733364, Vy = -2.1132827091217172, Vz = 0.11385995501982366)
Time since epoch: 2000000 min
Position (km): (x = 11054.486932923917, y = -40658.27635624185, z = 1635.5945933881073)
Velocity (km/s): (Vx = 2.964198038268495, Vy = 0.8099990524788586, Vz = 0.1032917723914456)
Time since epoch: 2500000 min
Position (km): (40593.56553348237, y = -11134.784332207368, z = 2486.8607168434683)
Velocity (km/s): (Vx = 0.8175136377497744, Vy = 2.96286742092495, Vz = -0.07841253986503455)
Time since epoch: 3000000 min
Position (km): (x = 30549.12556750274, y = 29063.493629407978, z = 162.3154320499707)
Velocity (km/s): (Vx = -2.1123431811282027, Vy = 2.221533024821393, Vz = -0.23683632574840144)
Time since epoch: 3500000 min
Position (km): (x = -9055.922500393144, y = 41047.65542346771, z = -3310.073213239884)
Velocity (km/s): (Vx = -2.9972566649957533, Vy = -0.672256949943488, Vz = -0.1351650134962626)
Time since epoch: 4000000 min
Position (km): (x = -39881.614167309, y = 13207.092316204795, z = -3580.7279528653353)
Velocity (km/s): (Vx = -0.9793148013533146, Vy = -2.909201062829499, Vz = 0.17800703001792312)
@weatherlym15 @tikhonovits thank you for your patience, I have been very busy with work and travel. If you guys don't mind, ping me on Wednesday (march 13), I should be able to set aside some time to investigate this issue.
Dear @shashwatak,
as you see I was a little bit unpatient...!
Fortunately, I found the source of the progressive deviation between js and c++ versions.
By comparing all members of satrec
between the two versions, I found the following values as different wrt to the js version:
satrec.del1 -6.3969223360555608e-13 double
satrec.del2 1.4103037029390144e-11 double
satrec.del3 1.9783041987769300e-12 double
satrec.irez 1 int
satrec.xfact -0.0043750247186120847 double
satrec.xlamo 0.75935747679288923 double
satrec.xli 0.76396171705721150 double
satrec.xni 0.0043746456895873351 double
The property that makes the difference of course is irez
, which for some reason in the js version is equal to zero. After going one step further, I realized that the two ifs in "deep space initialization" section within the dsinit function (lines 2507-2515) do not work as expected. Hence, by substituting these if conditions by the way they are written in the c++ version, i.e.,
// -------------------- deep space initialization ------------
irez = 0;
if ((nm < 0.0052359877) && (nm > 0.0034906585))
irez = 1;
if ((nm >= 8.26e-3) && (nm <= 9.24e-3) && (em >= 0.5))
irez = 2;
everything worked fine! All position and velocity results between js and c++ versions now perfectly match.
I suggest to further substitute all if conditions, which now are written as the problematic ones, with the original ones in the c++ version to be sure that all conditions will be working properly under different settings.
Hope that helped.
Best, Nikos
@tikhonovits If you would be so kind as to make a PR with the necessary fixes, I will gladly accept it :) Please make sure the tests in the tests directory still pass. Also, please add your name to the contributors list end of the Introduction in the README.md :)
I also found this issue last night migrating from RequireJS to CommonJS. ESLint pointed to these lines and I left them as is because wasn't sure how to fix them properly.
@shashwatak, if you feel that code refactoring proposed by pull request mentioned above is acceptable, I can create another pull request later to fix this issue (like @tikhonovits proposed) and add some test cases based on Astra 2F results from C++ version.
@ezze @tikhonovits are there any open PRs to address this? I’d be happy to open one myself but don’t want to take credit for anyone else’s work.
@dangodev, I decided to not make any changes to this repo until this big pull request is merged or rejected. Seems that @shashwatak is not supporting the repo anymore.
@ezze That makes sense. I looked through that and saw you hadn’t sneakily changed the values for src/dsinit.js
.
Is it time to discuss forking this repo?
@dangodev +1 for forking. But probably a person having a good knowledge in orbital mechanics is required to support it further.
But probably a person having a good knowledge in orbital mechanics is required to support it further.
hides under desk
@dangodev @ezze If any help will be needed with orbital mechanics, I will be happy to contribute. At least the bug mentioned in my previous post must be corrected.
I am sorry I’ve been unable to give any time to this project :(
I just made @ezze a Collaborator on this project to hopefully unblock everybody.
On Mon, Nov 13, 2017 at 12:23 Nikos Sagias notifications@github.com wrote:
@dangodev https://github.com/dangodev @ezze https://github.com/ezze If any help will be needed with orbital mechanics, I will be happy to contribute. At least the bug mentioned in my previous post must be corrected.
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/shashwatak/satellite-js/issues/26#issuecomment-344046128, or mute the thread https://github.com/notifications/unsubscribe-auth/AAoczq5o0O8HDY1uzhKEyD9BJ9T4wQ1bks5s2KU7gaJpZM4LuFMI .
@shashwatak That’s good news! Glad to hear it.
And no need to apologize! You’ve done some incredible work on this project that you should be very proud of. Nothing to be sorry for. But all in all, glad to hear your good work can continue moving forward.
@shashwatak No worries, we all have lives! Appreciate you giving @ezze collaborator rights to keep this awesome project going. :)
Hey, @shashwatak! Great to hear you again! :) Thank you so much for your trust, I accepted your inventation. Hope that we will be able to move this project forward.
@shashwatak, one very important thing is to gain an access to managing of NPM package, otherwise I have no chances to publish new versions of the library to NPM. As I can see, npm owner command is meant for that. Currenly, only you is able to manage the package:
$ npm owner ls satellite.js
shashwatak <shashwatak@gmail.com>
I guess that you should run something like this to grant me the access:
$ npm owner add ezze satellite.js
I also want to have Travis CI support here but, unfortunatelly, I can't enable it as collaborator. @shashwatak, could you, please, enable it for this repository?
In order to not pollute this issue with common talks on this repo, what do you think, guys, about moving further discussions to, say, Gitter? @shashwatak, can you enable a chat for this repository there?
Probably, we will want to have a coverage for tests in the future, fortunatelly, I had a luck to enable Coveralls by myself.
First of all, I plan to merge my recent pull request related to Common.js support already mentioned above and update dependency libraries. I hope that you all, guys, @dangodev @tikhonovits @davidcalhoun give it a try and tell me if there is something wrong with it. I'll try to do it today.
I also think of creating new branches, at least a dev
branch to have some git workflow here. Making new dev
branch as default we will force collaborators contributors cloning the repo to rebase their feature branches to this dev
branch and leave master
branch clean for releases only.
After all, @dangodev or any other, can fix this issue and make a pull request. :)
5th and 6th of the previous comment are done. @dangodev, you can give a try to fix this issue but before that let's discuss a Git workflow to use (I propose to branch your pull request from develop
branch). See #35.
@ezze all that sounds good to me. I’ll give your CJS branch a poke later this week. As for the workflow, that all works for me. I’ve commonly seen it both ways—either master
is the stable version, or master
is the latest (unstable) version. I don’t have an opinion one way or the other; we can go with what you proposed:
master
is current stable version
- develop
is the latest, in-progress development versionAny PRs are forked from develop
and re-merged there.
I’m also open to using Gitter. I’ve never tried it before, but I’ll give it a shot.
But back to this issue, I can open up a PR this week and try to implement @tikhonovits’ patch unless someone else beats me to it. I’ll compare it to the target values and hopefully we’ll be able to hit those with just this one patch.
@dangodev, please write some tests relying on input data and results provided by @tikhonovits in your pull request. Probably adding these values to existing test/sgp4.json
file will be enough.
As for workflow, let's wait for what @shashwatak says. If he leaves master
branch as the default one then it will remain a development branch. The most important things now are to gain access to NPM package and to enable Travis CI.
TravisCI support is now enabled, i believe Gitter should be up as well WIP to grant ownership of npm package to @ezze
@ezze you now have npm ownership :)
I also went ahead and protected both master
and develop
from deletion and force push, including from myself. I have a bad tendency to amend commits and force push sometimes 😬 this should put an end to that :)
@shashwatak, Travis CI is configured for the repo. It lints, tests and builds the code for each new commit in master
and develop
branches.
Gitter chat is available, everyone interested can join it now (the link is added to README.md
).
I also see myself in contributors' list of the npm package.
@dangodev, start your pull request to fix this issue branching from develop
.
Fixed by #37.
The fix is finally released in 2.0.0. You can try to update the library and give it a try.
I followed what you have in your README.md where I made timeSinceTleEpochMinutes = increments of 300.
var tleLine1 = '1 25544U 98067A 13149.87225694 .00009369 00000-0 16828-3 0 9031', tleLine2 = '2 25544 051.6485 199.1576 0010128 012.7275 352.5669 15.50581403831869'; var satrec = satellite.twoline2satrec(tleLine1, tleLine2); var positionAndVelocity = satellite.sgp4(satrec, timeSinceTleEpochMinutes);
But when I print out the x, y, and z positions and velocity values, the values differ from what I get using the python library you said you copied your satellite-js library from. At first the differences are small but they become a problem as my 300 second increments get larger and larger
Am I missing a step?