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

Adding TheRealLecture1 file #14

Merged
merged 8 commits into from
Sep 9, 2024
Merged

Conversation

Hardit-Saini
Copy link
Contributor

Lecture 1 uploaded. Called it TheRealLecture1 as there was already a file called lecture1.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@angadhn
Copy link
Owner

angadhn commented Aug 30, 2024

@Emma-airi and @mitanshc can you both help review this please?

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, but minor formatting issues to bring it in line with other lectures

  • Remove “Lecture 1” heading
  • All vectors are denoted by a bold face. This can be done using ‘\mathbf{}’. This is applied throughout the lecture.


Reply via ReviewNB

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Vectors (Brief Recap)
  • Following with what Mitansh has mentioned, you can make this as the main title of the chapter. It is good as a first level heading.
  • This heading will also need to have its own markdown cell.
  • Lastly, you would need to add the list of authors below the title, for example:
# Vectors (Brief Recap)
Prepared by: [insert full name here](https://github.com/insert your username here) and [Angadh Nanjangud](https://www.angadhn.com/)

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why, but there seems to be an extra “/“ in front of every equation. Maybe this is how it was typed out? Or maybe just me?


Reply via ReviewNB

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More information on the equations throughout the notebook:

  • Write the equations on each line (not a new cell, just leave some space between the equations and other texts.) For example:
### Two-Body Relative Dynamics


#### Newton's Second Law
1. $ m_2 \ddot{\vec{r}}_2 = \vec{F}_{21} = -  \frac{G m_1 m_2 \vec{r}}{r^3}  \rightarrow  \ddot{\vec{r}}_2 = -  \frac{G m_1  \vec{r}}{r^3} $


2. $ m_1 \ddot{\vec{r}}_1 = \vec{F}_{12} =  \frac{G m_1 m_2 \vec{r}}{r^3} \rightarrow  \ddot{\vec{r}}_2 =  \frac{G m_2  \vec{r}}{r^3} $


Combining these equations:

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the first equation hasn’t worked. Again probably an extra set of characters somewhere or indentation.


Reply via ReviewNB

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with the first equation in this section as well


Reply via ReviewNB

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too.

Hardit, how are you formatting your equations? Is it like Angadh showed in the closed Lecture 7 PR? Because they should all be numbered and you shouldn’t be needing dollar signs (if memory serves right)


Reply via ReviewNB

@@ -0,0 +1,428 @@
{
Copy link
Collaborator

@mitanshc mitanshc Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with equations and I’m not sure if the html will render on the website or not, it certainly isn’t here.


Reply via ReviewNB

Copy link
Collaborator

  • All the references you have made would need to be sent to the end of the chapter, under its own 2nd level heading.
  • you can decide on a referencing style for your links as well. I have personally used Harvard referencing style.

Copy link
Collaborator

As part of a table of content section at the beginning of the note, you can do so by:

In this lecture we cover the following topics:
1. [](content:sun-synchronous-orbit)
2. [](content:j2-perturbations)
3. [](content:repeat-groundtrack-orbits)
4. [](content:geosynchronous-orbits)
5. [](content:geostationary-orbits-(GEO))
6. [](content:sunsynchronous-repeat-groundtrack-orbits)
7. [](content:molniya-orbits)
8. [](content:tundra-orbits)
9. [](content:references)

This is how the markdown code looks like for mine.

The content part corresponds to each heading you have used like this for the number 1 content in the list:

(content:sun-synchronous-orbit)=
## Sun Synchronous Orbit

using this format for all the titles will get you up to speed.

You can see how the table of content looks like on the online textbook here: https://www.angadhn.com/SpacecraftDynamics/orbital-mechanics/Lecture12/Lecture12.html#content-sun-synchronous-orbit

@angadhn
Copy link
Owner

angadhn commented Sep 5, 2024

@Hardit-Saini do these two things:

  • can you rename your file to be Lecture1.ipynb
  • Move this file and the images into the Lecture1 directory

@angadhn
Copy link
Owner

angadhn commented Sep 5, 2024

@Hardit-Saini note that you can just drag and drop all the image into the already existing images directory. Otherwise, the older images may get deleted. Thanks!

@angadhn
Copy link
Owner

angadhn commented Sep 5, 2024

@Hardit-Saini doesn't look like any of the changes recommended above by @mitanshc and @Emma-airi have been made in the new file. Am I missing something?

@Hardit-Saini
Copy link
Contributor Author

@angadhn @Emma-airi I just commited the new file a few minutes ago, could this please get checked?

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

@Hardit-Saini also, it looks like this branch is not upto date with my main branch; see image below which says you are 9 commits behind the angadhn/SpacecraftDynamics:main. So you will have to follow the instructions in Contributing.md, just to ensure your fork is updated to have the newest content.

image

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

@Hardit-Saini you have not implemented the changes from this comment of mine on this PR. Here is a link to it: #14 (comment)

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

  • Please remove the italics for the different notations of vectors.
    image
    For example, you can use ${\bf r}$ to get ${\bf r}$. You have done this correctly in the sentence right above Equation 4 but then again the equation has the wrong way of writing $r$. See below for what I mean:
    image

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

  • In the below write-up, you have $Q$ as a scalar, which is correct. But the equation itself seems to show a bold-face Q, which is a vector. Please make that also italicised. This is likely happening because you are using brackets incorrectly for \bf which is applying it to every term.
    image

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

  • Make $\hat B$ into $\hat {\bf B}$ as it is a vector.
    image

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

  • Make unit vectors bold
  • Figure needs to be inserted correctly as it is currently missing when I generate the online textbook. Please double check this.

image

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

  • Something isn't right here, as well. I believe that $\hat r$ should be in bold in both equations:
    image

@Emma-airi can you also review this entire Lecture once the revisions are made by @Hardit-Saini based on my comments above, please? Thanks!

@Hardit-Saini
Copy link
Contributor Author

@Hardit-Saini do these two things:

  • can you rename your file to be Lecture1.ipynb
  • Move this file and the images into the Lecture1 directory

Hello @angadhn , I dont understand what I am meant to do here. My file name is Lecture1 however my branch is called TheRealLecture1 (this is because theres already a branch called Lecture1 with content in it so I didnt want to intrude). Could this please be elaborated?

@Emma-airi
@thedukeofeelam

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

@Hardit-Saini no worries. Just make the changes suggested by @thedukeofeelam. And ensure that you've updated your fork before pushing your next commit.

@Emma-airi
Copy link
Collaborator

Emma-airi commented Sep 6, 2024

  • In the below write-up, you have
    Q
    as a scalar, which is correct. But the equation itself seems to show a bold-face Q, which is a vector. Please make that also italicised. This is likely happening because you are using brackets incorrectly for \bf which is applying it to every term.
    image

Good Evening,

I wanted to ask if the unit vectors should be in boldface as well, or is the hat just enough. For example:
\hat{bf{i}}
or
\hat{rm{i}
or
\hat{bm{i} (this makes i hat, italics)

@angadhn
Copy link
Owner

angadhn commented Sep 6, 2024

It should be consistent with the later notes- boldface minimum but I imagine we have the hats too. Just take a look at the textbook link that is live.

… updated either because I was getting error messages earlier, pls let me know
@Hardit-Saini
Copy link
Contributor Author

Hardit-Saini commented Sep 7, 2024

I worded the commit message very poorly, I meant the file was updated thanks to @Emma-airi however I encountered some issues during fork updating (all thanks to my own incompetency).

@angadhn
Copy link
Owner

angadhn commented Sep 8, 2024

@Emma-airi can you review this tomorrow and let me know if ready for merge?

@Emma-airi
Copy link
Collaborator

Emma-airi commented Sep 8, 2024

  • Either bullet points or a comma between the gravitational constant and unit vector r parameters, makes it neater.
image
  • With the two images below, it seems to be a duplicate, but I assume they were meant to be mini titles, so I would recommend meaning them bold by using the double ** on each end of the titles; or using bullet points + removing the double title.
    WhatsApp Image 2024-09-08 at 16 25 35_96bb0ab1
    WhatsApp Image 2024-09-08 at 16 25 35_922124c7

@Emma-airi
Copy link
Collaborator

I apologise for not spotting these on the files when we were first preparing them @Hardit-Saini. Other than these, I am more than happy for the file to be merged!

@Emma-airi
Copy link
Collaborator

@Emma-airi can you review this tomorrow and let me know if ready for merge?

Good afternoon @angadhn , I wanted to confirm if there is anything else you expected me to do concerning the merge apart from looking through the document. I am wondering if there is a tool I should have access to that allows me to view if the file is ready to be merged and I am not using it.

@angadhn
Copy link
Owner

angadhn commented Sep 8, 2024

@Emma-airi all good. Just reviews for now. Only I should do merging. It's kind of a new thing for me too so only I should be the point of failure there until I learn better :-)

thanks for the effort reviewing on a Sunday!

@angadhn
Copy link
Owner

angadhn commented Sep 9, 2024

@Hardit-Saini, have you addressed the comments by @Emma-airi ?

@angadhn
Copy link
Owner

angadhn commented Sep 9, 2024

@Hardit-Saini you will also need to make sure to update this again as I merged your other PR. You can see the fruits of your labour are now online: https://www.angadhn.com/SpacecraftDynamics/orbital-mechanics/Lecture10/Lecture10.html

@Hardit-Saini
Copy link
Contributor Author

@angadhn Sorry, I would like to check if I uploaded the right version of the file. Please give me a moment

@Hardit-Saini
Copy link
Contributor Author

Ok I just checked reviewNB and the most recent commit is the correct file and has the changes @Emma-airi requested. It can be merged now @angadhn and thank you.

@angadhn
Copy link
Owner

angadhn commented Sep 9, 2024

@Hardit-Saini Legend! Thanks again for all the work y'all. Celebration gathering sometime next week. @Emma-airi @mitanshc @Joosty @thedukeofeelam @nozzington @calan04

@angadhn angadhn merged commit b050034 into angadhn:main Sep 9, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants