Updated tutorial #6
Conversation
|
Hi @grantcurell, thanks a lot for the PR. I am working on some more tutorials & cleaner API, which hopefully will clarify some of your comments/doubts (this is currently happening here: #7 ). There will be several new tutorials that go through basic and more advanced functionalities of dSGP4, in particular:
Due to the large amount of changes, I will first push that PR, and then, if you do not mind, ask you to revise this, based on what is still outstanding from that. Cheers! |
|
@grantcurell #7 has been merged. Please, have a look, and let me know if some of the aspects you mentioned are solved. As for the rest: can you please update this PR to reflect those changes? Perhaps it is easier to close this, and re-open a new one? As you prefer! Thanks again. |
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
from
3517e2f to
2fbdd08
Compare
|
PR updated and tests run: |
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
from
2fbdd08 to
8456f08
Compare
|
Thanks! I left a few comments. Also, in case you want to run all the tests locally, you also need to build the docs if you want to mirror what happens in the CI (currently the docs are failing) |
|
I can't build locally because the code was written with dependencies on files I don't have. TLDR: |
|
@grantcurell this is gone after #7 (see: https://github.com/esa/dSGP4/blob/master/doc/notebooks/gradient_based_optimization.ipynb). I guess you have not updated this file? You should be able to build locally, but let me know if that is not the case :) |
My bad, I'll go back through it line by line |
hopefully it is not going to be too painful :) |
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
2 times, most recently
from
a749d0d to
662e808
Compare
Na, wasn't too bad. Let me know if I need to clean up anything else. I'm jazzed about the new tutorials - this is a cool project |
|
Looks like there is a failure across all the notebooks but it does build locally: Sphinx
|
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
from
662e808 to
fd7c474
Compare
|
Thanks for the followup @grantcurell It seems to be related to the .py file you pushed.. we could resolve it, but I think that file is not needed anyway (I would refrain to push things not strictly related to the project). I left a review above discussing this and a few other things, if that is okay, I would recommend addressing those first |
No problem - the python file just generates the graph visually demonstrating gradient descent with Newton's method. I can delete it. This is my ignorance showing - where is the review with the requested changes? |
|
I just tagged you there: let me know if you see it :) (you might also be able to access the review from the top right part of the PR, under the +++ --- of the lines of code) |
Apologies - busy at work. I checked my notifications and I don't have a tag - the only mention I have is on this thread:
Can you link it? Are these inline code comments? (Like this)
I didn't see any and normally those show up in the PR thread but maybe there is something I'm missing? Are you able to drop a hyperlink to it? |
|
@grantcurell just scroll up in this PR, and you should see: "Sceki requested a review" and all my comments still "pending". As for the hyperlink, maybe try this: https://github.com/esa/dSGP4/pull/6/files#diff-83f35df9a3fe0a2b91c93481a5af0169fd86ba7f4296ae84b9e7fa65ad1af9a2. Not sure it works. Let me know how it goes! |
|
Apologies, on the road a lot and been busy. I'm familiar with what that should look like from other projects, but I don't see it here and it's not visible on my page. I also don't see any of the reviewer comments you normally get when someone leaves feedback on the commit itself. You're talking about this right? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request I've looked everywhere I can think to look, but I don't see any of the comments like I normally would. Let me know if I'm just off in outer space. (No pun intended) |
|
@grantcurell Yes, I am talking about that. I also linked it: can you click and check if it works? |
|
(Here's an example of the comments I gave:) 
|
|
That's why - your comments are in the pending state. You haven't actually posted them yet. See this stack overflow post: https://stackoverflow.com/a/45336490/4427375 |
you are right. I needed to press the complete review button! Done now, let me know! |
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
2 times, most recently
from
79c6b2f to
8b4c56b
Compare
…g of what is going on with an explanation at a level that an engineer with a bachelors in comp sci understands - Added/updated comments for the gradient_based_optimization tutorial - Shamelessly added my name to the bottom of that tutorial so that I have something to point to for the time spend with leadership. Feel free to delete it if my explanation is terrible :-p - Added comments to initl.py Signed-off-by: Grant Curell [email protected]
grantcurell
force-pushed
the
update_gradient_descent_tutorial
branch
from
8b4c56b to
428d0c1
Compare
|
All requested changes made! |
There was a problem hiding this comment.
Now that the PR matches the master I could have a look and I went through the tutorial, but there is some confusion on the notebook content: we are leveraging the gradient to apply Newton's method here, we are not doing gradient descent.
Of course, there are tons of application where you can use dsgp4 for gradient descent, but in this case it was not used.
In the paper, we discuss for instance a case where we apply gradient descent to learn the ML-dSGP4 corrections.
Anyway, I cannot accept this PR as it is, since it creates a bit of confusion and it is incorrect w.r.t. what it's actually done in the tutorial. I am sorry and thanks a lot for spending the time. I will be happy to accept other contributions in the future (in case they add relevant content/explanations!).
| "\n", | ||
| "We can propagate the state from $t_0 \\rightarrow t_{obs}$, and obtain the state at $t_{obs}$. In general, we define the state (i.e., position and velocity), as:\n", | ||
| "We can use the SGP4 algorithm to propagate the state from $t_0 \\rightarrow t_{obs}$, and obtain the state at $t_{obs}$. We define the state (i.e., position and velocity) of an object in 3D space, as (this is described in more detail below):\n", |
There was a problem hiding this comment.
it is actually 6D (position and velocity), so please remove the 3D specification
| "- **$[ \\cdot ]$:** Indicates this is a vector\n", | ||
| "- **$x(t), y(t), z(t)$:** These are the positional coordinates of the object at time $t$. They represent the location of the object in a three-dimensional space\n", | ||
| "- **$\\dot{x}(t), \\dot{y}(t), \\dot{z}(t)$:** These terms represent the velocities of the object in the direction of each corresponding axis (X, Y, and Z). The dot above each symbol signifies these are first derivatives of the position coordinates. $\\dot{x}(t)$ is the velocity in the X direction, $\\dot{y}(t)$ is the velocity in the Y direction, and $\\dot{z}(t)$ is the velocity in the Z direction.\n", |
There was a problem hiding this comment.
we do not need these three explanations.. they are kind of trivial, and can be found in the paper anyway if someone wants to dig deeper
| "- **$x(t), y(t), z(t)$:** These are the positional coordinates of the object at time $t$. They represent the location of the object in a three-dimensional space\n", | ||
| "- **$\\dot{x}(t), \\dot{y}(t), \\dot{z}(t)$:** These terms represent the velocities of the object in the direction of each corresponding axis (X, Y, and Z). The dot above each symbol signifies these are first derivatives of the position coordinates. $\\dot{x}(t)$ is the velocity in the X direction, $\\dot{y}(t)$ is the velocity in the Y direction, and $\\dot{z}(t)$ is the velocity in the Z direction.\n", | ||
| "\n", | ||
| "We then have: TLE$_0$, $\\vec{x}(t_0)$, and $\\vec{x}(t_{obs})$, but we want to find TLE$_{obs}$. Said with an example: Imagine we have some satellite with TLE$_0$. For TLE$_0$ we have all the orbital parameters required to define the satellite's orbit at $t_0$; inclination, right ascension of the ascending node, eccentricity, etc. Now we use TLE$_0$ as input to an orbital propagation model (SGP4) to compute the satellite's state vector at time 0 ($t_0$). This calculation gives us $\\vec{x}(t_0)$ which is the satellite's position and velocity in space. Now fast-forward a bit into the future to a new time ($t_{obs}$) where we again observe the satellite's actual position and velocity ($\\vec{x}(t_{obs})$). The problem is that our initial estimate $\\vec{x}(t_0) \\neq \\vec{x}(t_{obs})$. Our estimate was wrong. What we now want to do is to calculate TLE${_{obs}}$. TLE${_{obs}}$ is the TLE set that when applied at our new observation time **would** give us $\\vec{x}(t_{obs})$. Said another way, we want to be able to invert from state to an accurate TLE that correctly produces TLE$_{obs}$.\n", |
There was a problem hiding this comment.
I think there is a misunderstanding here. The point is only that: we have TLE_0, which would result in \vec{x}(t_{obs}) when propagated at t_{obs}. We want to find the TLE_{obs} that corresponds to that \vec{x}(t_{obs})
As you explain it, it seems that our objective is to match \vec{x}(t_0) and \vec{x}(t_{obs}), which is not the case
| "\n", | ||
| "We then have: TLE$_0$, $\\vec{x}(t_0)$, and $\\vec{x}(t_{obs})$, but we want to find TLE$_{obs}$. Said with an example: Imagine we have some satellite with TLE$_0$. For TLE$_0$ we have all the orbital parameters required to define the satellite's orbit at $t_0$; inclination, right ascension of the ascending node, eccentricity, etc. Now we use TLE$_0$ as input to an orbital propagation model (SGP4) to compute the satellite's state vector at time 0 ($t_0$). This calculation gives us $\\vec{x}(t_0)$ which is the satellite's position and velocity in space. Now fast-forward a bit into the future to a new time ($t_{obs}$) where we again observe the satellite's actual position and velocity ($\\vec{x}(t_{obs})$). The problem is that our initial estimate $\\vec{x}(t_0) \\neq \\vec{x}(t_{obs})$. Our estimate was wrong. What we now want to do is to calculate TLE${_{obs}}$. TLE${_{obs}}$ is the TLE set that when applied at our new observation time **would** give us $\\vec{x}(t_{obs})$. Said another way, we want to be able to invert from state to an accurate TLE that correctly produces TLE$_{obs}$.\n", | ||
| "\n", | ||
| "This is where gradient descent comes into play. If you have no prior experience with gradient descent this is very likely going to be confusing. You can read more about it [here](https://www.khanacademy.org/math/multivariable-calculus/applications-of-multivariable-derivatives/optimizing-multivariable-functions/a/what-is-gradient-descent). The very high level description is that gradient descent is an optimization algorithm used to minimize a function by iteratively moving in the direction of the steepest decrease, as defined by the negative of the gradient. We use it in machine learning to iteratively refine model parameters, aiming to find the set of parameters that minimizes the cost function, typically representing the discrepancy between predicted and observed data. \n", |
There was a problem hiding this comment.
This is actually Newton's method, not gradient descent. It still uses the gradient, but it's formulated differently: https://en.wikipedia.org/wiki/Newton%27s_method (we are seeking F(x)=0 here)
| "\n", | ||
| "This is where gradient descent comes into play. If you have no prior experience with gradient descent this is very likely going to be confusing. You can read more about it [here](https://www.khanacademy.org/math/multivariable-calculus/applications-of-multivariable-derivatives/optimizing-multivariable-functions/a/what-is-gradient-descent). The very high level description is that gradient descent is an optimization algorithm used to minimize a function by iteratively moving in the direction of the steepest decrease, as defined by the negative of the gradient. We use it in machine learning to iteratively refine model parameters, aiming to find the set of parameters that minimizes the cost function, typically representing the discrepancy between predicted and observed data. \n", | ||
| "\n", | ||
| "So how does gradient descent apply here? We can reformulate our problem, that is, how do we take $\\vec{x}(t_0)$ (that's the estimation made by SGP4) and get it so that it is as close as we can to $\\vec{x}(t_{obs})$ (the actual position of an object at time $t_{obs}$). This is where gradient descent comes in. We create a cost function:\n", |
There was a problem hiding this comment.
this is incorrect in our case, we are not updating x(t_0) to match x(t_obs), we are instead looking for the TLE at t_obs with parameters (called y) so that TLE_obs produces a state x(t_obs) that matches the one found from TLE_0. I will try to add a few sentences to make this clearer in the tutorial :)
| "- $no_{kozai}$: The mean motion of the satellite, adjusted for the Kozai correction. This represents the number of orbits the satellite completes in a day, adjusted for long-term perturbations in the orbit.\n", | ||
| "- $ecco$: The eccentricity of the orbit. This value defines the shape of the satellite's orbit, ranging from 0 (a perfect circle) to values close to 1 (highly elliptical orbits).\n", | ||
| "- $inclo$: The inclination of the orbit, measured in degrees. It indicates the angle between the satellite's orbital plane and the equatorial plane of the Earth.\n", | ||
| "- $mo$: The mean anomaly at the epoch. This is an angular measurement that specifies the satellite's position along its orbit at the specific time defined by the epoch of the TLE set.\n", | ||
| "- $argpo$: The argument of perigee. This angle indicates the orientation of the elliptical orbit in relation to the Earth's surface, specifying the point where the satellite passes closest to the Earth.\n", | ||
| "- $nodeo$: The right ascension of the ascending node (RAAN). This is the angle from a fixed reference direction, typically the vernal equinox, to the location where the satellite crosses the equatorial plane going northward.\n", | ||
| "- $n_{dot}$: The first derivative of the mean motion. It indicates how the satellite's mean motion changes over time, which is primarily due to atmospheric drag and gravitational perturbations.\n", | ||
| "- $n_{ddot}$: The second derivative of the mean motion. This value provides a refinement on the rate of change in the satellite's mean motion, offering a more precise prediction of its long-term orbital behavior.\n", | ||
| "- $B^*$: The ballistic coefficient. It relates to how the satellite responds to atmospheric drag, with a higher value indicating a greater effect of drag on the satellite's orbit.\n", |
There was a problem hiding this comment.
I already go over these in a separate tutorial: TLE Object
|
No problem! I really appreciate your taking the time to write the feedback. I will read through it, read the paper again, go study some more, and try again! This has been a very interesting free time project. |
|
Thanks! Looking forward to it :) And closing this for now :) |
I went looking for ways to improve on the SGP4 algorithm and stumbled across your paper. I'm only sitting on a bachelors in comp sci & engineering so it took me a bit to spin up on everything and get to a point where it made sense to me. I overhauled the gradient descent tutorial so that it will make more sense at a lower education level and make the project more accessible to us average people hahaha.
Feel free to kick it back to me if there's anything you don't like and I'll fix it.
Signed-off-by: Grant Curell [email protected]