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

Rk4 sim #41

Merged
merged 5 commits into from
Feb 15, 2024
Merged

Rk4 sim #41

merged 5 commits into from
Feb 15, 2024

Conversation

PatXue
Copy link
Collaborator

@PatXue PatXue commented Feb 15, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Change physics simulator step function to use RK4 instead of midpoint rule for improved simulation accuracy.

QA Instructions, Screenshots, Recordings

Ran simulator on Gabriel's laptop and buggy appeared to travel as expected. Simulator ran very slowly (10Hz), but doesn't seem to be related to these changes, since even after undoing all the changes it ran at the same speed.

@PatXue
Copy link
Collaborator Author

PatXue commented Feb 15, 2024

Why does pylint think this commit changed every file in rb_ws?

@christianvluu
Copy link
Collaborator

Why does pylint think this commit changed every file in rb_ws?

Can you try pulling in/merging in main? Also which branch did you guys start from?

@PatXue
Copy link
Collaborator Author

PatXue commented Feb 15, 2024

Why does pylint think this commit changed every file in rb_ws?

Can you try pulling in/merging in main? Also which branch did you guys start from?

Ah, we started from main 2 weeks ago, and there's been some changes so that's probably why

@christianvluu
Copy link
Collaborator

Okay I’m feeling line endings change? @Jackack had this issue before so perhaps he can diagnose if that’s what causing this.

@PatXue
Copy link
Collaborator Author

PatXue commented Feb 15, 2024

Okay I’m feeling line endings change? @Jackack had this issue before so perhaps he can diagnose if that’s what causing this.

Is that the LF/CRLF thing in the bottom right? I have it set to LF

@Jackack
Copy link
Collaborator

Jackack commented Feb 15, 2024

If you look at the pylint output for each of the commits in this branch, pylint only complained about engine.py until the "fix pylint" commit, which caused pylint to complain about a bunch of other files.

@Jackack
Copy link
Collaborator

Jackack commented Feb 15, 2024

Just a hunch, might be merging the SW/Ui velocity PR from main that caused this

@Jackack
Copy link
Collaborator

Jackack commented Feb 15, 2024

also what did you do in the merge branch main commit? This branch is still behind main. Did you pull main from origin first when you merged?

@Jackack
Copy link
Collaborator

Jackack commented Feb 15, 2024

I took a look at the git log of this branch.

Since pylint checks the files that are different from main, I think what happened is:

  1. main got updated because PRs were merged
  2. you pushed this morning, without merging main into your branch
  3. because your branch has files from an older version of main, pylint sees them and thinks: these files are different from the current main, let's check them
  4. because these files are from before pylint existed and don't have proper style, pylint complains.
  5. you merge main into your code, but without pulling main first. Your local main branch has the ui velocity PR, but not the auton overtake PR.
  6. the files that pylint were mad about are still behind main, so pylint still complains

The fix would be to pull from main, and then merge main into your branch again.

@PatXue
Copy link
Collaborator Author

PatXue commented Feb 15, 2024

I took a look at the git log of this branch.

Since pylint checks the files that are different from main, I think what happened is:

  1. main got updated because PRs were merged
  2. you pushed this morning, without merging main into your branch
  3. because your branch has files from an older version of main, pylint sees them and thinks: these files are different from the current main, let's check them
  4. because these files are from before pylint existed and don't have proper style, pylint complains.
  5. you merge main into your code, but without pulling main first. Your local main branch has the ui velocity PR, but not the auton overtake PR.
  6. the files that pylint were mad about are still behind main, so pylint still complains

The fix would be to pull from main, and then merge main into your branch again.

Ah ok it seems to have worked now after pulling and merging again

@Jackack
Copy link
Collaborator

Jackack commented Feb 15, 2024

Tested on my laptop, sim frequency is good. Approved.

@PatXue PatXue merged commit 3f71018 into main Feb 15, 2024
3 checks passed
@PatXue PatXue deleted the RK4-Sim branch February 15, 2024 16:34
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.

None yet

4 participants