angadhn / SpacecraftDynamics

Executable Textbook for Spacecraft Dynamics
http://www.angadhn.com/SpacecraftDynamics/
4 stars 11 forks source link

Lecture2 #10

Closed calan04 closed 1 month ago

calan04 commented 1 month ago

First pull request for lecture 2

review-notebook-app[bot] commented 1 month ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

angadhn commented 1 month ago

Here are the changes you need to make for this file:

review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

angadhn commented on 2024-08-27T14:40:51Z ----------------------------------------------------------------

Replace \bm with \bf (already commented elsewhere, as well)


review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

angadhn commented on 2024-08-27T14:40:52Z ----------------------------------------------------------------

Remove the red colour in the text for the "first" in "first order equations".


review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

angadhn commented on 2024-08-27T14:40:52Z ----------------------------------------------------------------

You can see that the angular momentum of $m_2$ with respect to $m_1$ is written with a boldface H. That is the correct way of writing vectors. So please use that boldface for vectors.


review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

angadhn commented on 2024-08-27T14:40:53Z ----------------------------------------------------------------

You have a line that says

1 + 2

Can you please look into how others have assigned equation numbers to their equations? Use it on the relevant equations here. If you want a full tutorial on numbering equations and referencing them please see:

Use the first link to only number the equations that correspond to 1 and 2 in the notes. Then you will be able to use the second link's instructions to make use of the label name anywhere else.


angadhn commented 1 month ago

@Joosty review support requested here, as well. I've also left some comments here that you can build off of. Again, your take is to just review, not make changes.

And maybe even helping @calan04 with the branching and PRs over a call, if you can make some time as @mitanshc is travelling but has made an excellent video on Teams.

I know @calan04 has a Lecture8 branch in her fork but she hasn't opened a PR for yet. I imagine it's because she needs brushing up on how branching works on her PC and how that links to GitHub. If I can support, let me know.

Joosty commented 1 month ago

Okay that sounds good. I can get on with a review of this PR this evening or tomorrow morning. I'm also happy to help @calan04 with any issues in terms of branching tomorrow if they need more help after the video - either teams or whatever works. :)

Joosty commented 1 month ago

Apologies that I didn't get this together last night, family commitments came up. :)

Summary of issues

I'm going to aggregate all the problems that myself and @angadhn have found into one list so it is easier to fix, I hope this helps and if you need any support I'm here to help. If I spot any more I'll continue this chain :)

Issues:

  1. Please replace all \bm with \bf. Vectors need to be non-italicized. image

  2. You have no images in the subdirectory titled images/. They are also not correctly implemented in the ipynb file. image

  3. Some of the equations have rogue '$' symbols. It looks as if you wanted a centred equation but instead ended up with inline equations. image

  4. The lecture should be named after the topic being covered.
    image

  5. You have used '1+2' to convey an equation number of some kind. I used '\tag{number}' to do equation numbers. image

  6. The page could be made more reader-friendly. Unfortunately, I don't follow the flow of the whole text, it bounces betteen points with not much flow. If I was learning this for the first time I would prefer it was presented in a step-by-step approach with explanations of what each step is doing. Perhaps look at lecture 5 or others for some guidance on this.

calan04 commented 1 month ago

Thanks very much!

From: Joosty @.> Date: Saturday, 31 August 2024 at 06:32 To: angadhn/SpacecraftDynamics @.> Cc: Ceyda Alan @.>, Mention @.> Subject: Re: [angadhn/SpacecraftDynamics] Lecture2 (PR #10)

Apologies that I didn't get this together last night, family commitments came up. :)

Summary of issues

I'm going to aggregate all the problems that myself and @angadhnhttps://github.com/angadhn have found into one list so it is easier to fix, I hope this helps and if you need any support I'm here to help. If I spot any more I'll continue this chain :)

Issues:

  1. Please replace all \bm with \bf. Vectors need to be non-italicized. image.png (view on web)https://github.com/user-attachments/assets/b0c9e8a0-c8ab-438e-8286-4c49cdbc13d3
  2. You have no images in the subdirectory titled images/. They are also not correctly implemented in the ipynb file. image.png (view on web)https://github.com/user-attachments/assets/47bef21d-9bd3-42ba-aee2-469b89af6f30
  3. Some of the equations have rogue '$' symbols. It looks as if you wanted a centred equation but instead ended up with inline equations. image.png (view on web)https://github.com/user-attachments/assets/2c3a4ace-0dd2-47da-b51f-b6d7425a74ae
  4. The lecture should be named after the topic being covered. image.png (view on web)https://github.com/user-attachments/assets/3a352534-6830-4aad-bd4e-867e09d0a724
  5. You have used '1+2' to convey an equation number of some kind. I used '\tag{number}' to do equation numbers. image.png (view on web)https://github.com/user-attachments/assets/8d2d93bd-919d-4cca-8a27-0dd28501d9c4
  6. The page could be made more reader-friendly. Unfortunately, I don't follow the flow of the whole text, it bounces betteen points with not much flow. If I was learning this for the first time I would prefer it was presented in a step-by-step approach with explanations of what each step is doing. Perhaps look at lecture 5 or others for some guidance on this.

— Reply to this email directly, view it on GitHubhttps://github.com/angadhn/SpacecraftDynamics/pull/10#issuecomment-2322855701, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BJTTXPMEAC3UAVNBDA3EFUTZUGLUVAVCNFSM6AAAAABMQUE2QSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRSHA2TKNZQGE. You are receiving this because you were mentioned.Message ID: @.***>

calan04 commented 1 month ago

Hi,

Thanks for putting together a summary of all that is to be done @Joost @.***>.

I have a few questions about some of the issues. You mention that the images are not correctly implemented in the ipynb files, what would be the correct implementation?

Regarding the $ symbol, I have intentionally used in line equations, would you like me to remove them?

Concerning the final point, my understanding was that we had to copy everything off the slides as it is. I’m not sure how I can change the files to add more flow.

Thanks,

From: Joosty @.> Date: Saturday, 31 August 2024 at 06:32 To: angadhn/SpacecraftDynamics @.> Cc: Ceyda Alan @.>, Mention @.> Subject: Re: [angadhn/SpacecraftDynamics] Lecture2 (PR #10)

Apologies that I didn't get this together last night, family commitments came up. :)

Summary of issues

I'm going to aggregate all the problems that myself and @angadhnhttps://github.com/angadhn have found into one list so it is easier to fix, I hope this helps and if you need any support I'm here to help. If I spot any more I'll continue this chain :)

Issues:

  1. Please replace all \bm with \bf. Vectors need to be non-italicized. image.png (view on web)https://github.com/user-attachments/assets/b0c9e8a0-c8ab-438e-8286-4c49cdbc13d3
  2. You have no images in the subdirectory titled images/. They are also not correctly implemented in the ipynb file. image.png (view on web)https://github.com/user-attachments/assets/47bef21d-9bd3-42ba-aee2-469b89af6f30
  3. Some of the equations have rogue '$' symbols. It looks as if you wanted a centred equation but instead ended up with inline equations. image.png (view on web)https://github.com/user-attachments/assets/2c3a4ace-0dd2-47da-b51f-b6d7425a74ae
  4. The lecture should be named after the topic being covered. image.png (view on web)https://github.com/user-attachments/assets/3a352534-6830-4aad-bd4e-867e09d0a724
  5. You have used '1+2' to convey an equation number of some kind. I used '\tag{number}' to do equation numbers. image.png (view on web)https://github.com/user-attachments/assets/8d2d93bd-919d-4cca-8a27-0dd28501d9c4
  6. The page could be made more reader-friendly. Unfortunately, I don't follow the flow of the whole text, it bounces betteen points with not much flow. If I was learning this for the first time I would prefer it was presented in a step-by-step approach with explanations of what each step is doing. Perhaps look at lecture 5 or others for some guidance on this.

— Reply to this email directly, view it on GitHubhttps://github.com/angadhn/SpacecraftDynamics/pull/10#issuecomment-2322855701, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BJTTXPMEAC3UAVNBDA3EFUTZUGLUVAVCNFSM6AAAAABMQUE2QSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRSHA2TKNZQGE. You are receiving this because you were mentioned.Message ID: @.***>

Joosty commented 1 month ago

Hi @calan04, I hope all is well! Happy to outline some solutions.

  1. Images: The main issue I can see here is that you have no images in the images file associated with this lecture. When using ipynb files it is best practice to store the images in a folder which you can then refer to and import in the file itself ( have put a screenshot of the file below). image In the past, I have used the method shown in the second screenshot to import the images. This involves giving the path to the image in question. There are other ways I've seen looking at other contributors here - look into this a bit yourself perhaps. image

  2. The rogue $ signs: The issue here is that you are using double $$ (which signifies non-inline equations) in an inline format since it is attached to a bullet point. Leaving a gap between the lines or just using inline equations (a single $) could fix this.

  3. Flow: This may be more of a question for @angadhn to answer but I was under the impression there was a certain amount of creative liberty that can be used to improve the lecture slides since they were designed to be talked over as well. I think it would help to explain the concepts and what is going on step by step - perhaps just a few sentences to help with the bullet points.

I hope this helps and I didn't ramble too much!

Cheers, Joost.