-
Notifications
You must be signed in to change notification settings - Fork 3
FiringSolutionSolver.java #22
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
Conversation
|
Are you ready for review? |
|
ok I'm ready I think |
mstatistics
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left comments for general practice things. However, we talked about using real world data and finding the best fit. We said that it is impossible to characterize the trajectory with high school physics equations. I might be missing something, but I don't see that here. It looks like you are still attempting to do perfect world math. I would be more ok with it if we could verifiably measure every value. How do we find the Drag coefficient for example? how do we know that it is wrong?
src/main/java/frc/robot/generic/subsystems/drive/DriveConstants.java
Outdated
Show resolved
Hide resolved
| // ---------------------- | ||
| // Tunable physical constants (dashboard-adjustable) | ||
| // ---------------------- | ||
| private static final LoggedTunableNumber kDragCoefficient = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add units to these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add it to the logging path? FiringSolver/VelocityTolerancemps for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you please elaborate on that a bit more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mstatistics when u have time ofc
|
uh so your suggested code broke the build.... |
|
Yah I just saw that. Turns out that is python syntax my apologies. Use what you had before |
|
also as for how it is real life not physics, my python code is almost done... |
Is it ok if i explain tomorrow when my python is finished? also, i mean this nicely, we cant make it data driven until we have data, and specifically, our data. these equations, which are mostly from a classical mechanics course textbook i read, are tuned and made data driven from data collected. we log what they do and if they work then my python tells me how to tweak it based off real data. just like you have to plug in a number to see how off you are, same thing here. the equations can and will be modified, but first they have to exist and be tested to see how to modify them. |
|
finished changes |
This reverts commit d8f4f8e.
|
just an fyi I rebased my fork to sync to main so that's why all my commits look like I just made them |
mstatistics
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the minor stylistic issues where fixed, nice!
However, this code still has the flaw that it assumes a perfect environment. Can you implement the 'find best solution of known working combinations' that we talked about on slack and that other teams used? This is not the sort of problem where we should try to calculate air resistance ourselves, for example.
src/main/java/frc/robot/generic/subsystems/drive/DriveConstants.java
Outdated
Show resolved
Hide resolved
@mstatistics can u elaborate a bit more on what you mean by this? |
…on, remove redundancy Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
…y, production tested Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
Co-authored-by: Ruthie-FRC <238046543+Ruthie-FRC@users.noreply.github.com>
…-folder Consolidate bayesopt tuner files into organized root folder structure
|
oh yeah uhh @mstatistics heres all the bayesopt stuff! (ignore the copilot i forgot folders existed and then was too lazy to do it myself) |
|
ok maybe i also added the bayesopt |
Corrected grammatical errors in the Master Developer Document.
|
apoloiges for any major issues with this, as i am sure that they exist |
Clarify instructions for enabling the tuner.
Removed sleep command before echo message.
fixed formatting and issues with non ascii characters
|
@mstatistics I'm closing this pull request. later on, I will make a seprate one with just the firingsolutions.java file. |
I made FiringSolutionSolver.java.