-
Notifications
You must be signed in to change notification settings - Fork 613
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
[wpimath] Add simulated annealing #5961
[wpimath] Add simulated annealing #5961
Conversation
6e6a936
to
79f88d6
Compare
The circular buffer changes are there to make the tests easier to write, and we've wanted a more generic circular buffer class for a while. That was a breaking change though to make it match how we organized the interpolating tree maps. The traveling salesman tests take 15 ms on my machine, which is rather slow given the size of the problem. I haven't looked into better convergence criteria. Part of the issue is you need to let the randomness stew a bit so the solver has a chance to reach a more optimal solution. |
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 it be worth splitting the CircularBuffer changes into a different PR (e.g. for commit history)?
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/path/TravelingSalesman.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/path/TravelingSalesman.java
Outdated
Show resolved
Hide resolved
wpimath/src/test/java/edu/wpi/first/math/optimization/SimulatedAnnealingTest.java
Outdated
Show resolved
Hide resolved
wpimath/src/test/java/edu/wpi/first/math/path/TravelingSalesmanTest.java
Outdated
Show resolved
Hide resolved
wpiutil/src/test/java/edu/wpi/first/util/DoubleCircularBufferTest.java
Outdated
Show resolved
Hide resolved
Depends on #5969. |
340d013
to
90e79b8
Compare
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.
Looks good to me (for what it's worth), just a few things that escaped the renaming.
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/java/edu/wpi/first/math/optimization/SimulatedAnnealing.java
Outdated
Show resolved
Hide resolved
wpimath/src/main/native/include/frc/optimization/SimulatedAnnealing.h
Outdated
Show resolved
Hide resolved
wpimath/src/main/native/include/frc/optimization/SimulatedAnnealing.h
Outdated
Show resolved
Hide resolved
0f91d7f
to
f94da27
Compare
Co-authored-by: Ashray._.g <ashray.gupta@gmail.com>
f94da27
to
d5731a0
Compare
Co-authored-by: Ashray._.g <ashray.gupta@gmail.com>
Co-authored-by: Ashray._.g ashray.gupta@gmail.com