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

Feedback code 2 #2

Open
11 tasks
mstoelzle opened this issue Aug 10, 2020 · 0 comments
Open
11 tasks

Feedback code 2 #2

mstoelzle opened this issue Aug 10, 2020 · 0 comments
Assignees
Labels

Comments

@mstoelzle
Copy link
Contributor

mstoelzle commented Aug 10, 2020

  • Please format the readme a bit more nicely

Screenshot 2020-08-10 at 16 34 45

  • It is not advisable to use spaces in filenames especially of executable files. It is advised to use _ instead of spaces
  • Please add a requirements.txt file in which you specify all necessary pip packages including the version number. This is an example:
control==0.8.3
Django==3.0.7
djangorestframework==3.11.0
django-webpack-loader==0.7.0
gitpython==3.1.1
psycopg2-binary==2.8.4
thrustcurve==0.1.0
gitdb2==4.0.2
numpy==1.18.4
matplotlib==3.2.1
pandas==1.0.3
scipy==1.4.1
  • Please specify in the README which Python version needs to be used (for example 3.8)
  • In the plot (I attached the screenshot below) can you please add proper labeling of the axis (x and y) and a plot title

Screenshot 2020-08-10 at 16 53 59

  • You need to find a solution to not commit the OWM secret key to github as it is a public repository and somebody could steal your key. I would propose to put it into a text file, which you dont push to github (you can add this textfile subsequently to the .gitignore file)
  • I you assuming a constant wind right now (I see in the code that you are accessing the wind from the api). Could you please do a literature survey on models how the wind changes with altitude and add this dynamic wind depending on altitude?
  • It is a convention to always use english in the code and all code comments
  • I think you ascent model is not really accurate. To be honest, I would leave it completely out as its in my opinion out of scope of this thesis. I would use a constant horizontal velocity at apogee (and obviously zero vertical velocity), with the rocket completely gravity turned (e.g. its flying horizontally) as a starting point for your descent model
  • Can you please add more nicely labeled plots so I / somebody else can better understand what your model is doing? I would think about the orientation of the rocket, velocities in all directions (both in the rocket and world frame) and position in the world frame and acceleration in the rocket frame. Please also add plots for the wind dependent on altitude.
  • You should start thinking about a more accurate parachute model (e.g. first a drogue parachute is released and later the main parachute)
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

2 participants