Resolves #587 - Implement the Shubert-Piyavskii method for minimization#588
Resolves #587 - Implement the Shubert-Piyavskii method for minimization#588Luis-Varona wants to merge 1 commit intoargmin-rs:mainfrom
Conversation
d097263 to
90d8204
Compare
|
Just added a minor bugfix to one of the doctests (didn't affect the actual implementation, though)--squashed into the same commit to avoid clutter. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #588 +/- ##
==========================================
+ Coverage 92.13% 92.17% +0.04%
==========================================
Files 177 178 +1
Lines 23672 24139 +467
==========================================
+ Hits 21810 22250 +440
- Misses 1862 1889 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
90d8204 to
c99ead9
Compare
|
One last bugfix--a spelling correction ("arbitraily" -> "arbitrarily") in the examples file and making the PartialOrd impl for SearchInterval more idiomatic in the main solver implementation. Should pass all CI tests now! 🙂 |
c99ead9 to
4bf9bf2
Compare
The Shubert-Piyavskii method optimizes a univariate, Lipschitz continuous function inside a specified interval. Given an accurate Lipschitz constant, it is guaranteed to come within an arbitrary epsilon of the true global minimum. Unlike most other deterministic global optimizations algorithm (e.g., golden-section search), it is not restricted to unimodal functions; moreover, it does not require an initial guess. It deterministically samples points from ever-narrowing subintervals of the search space until the sampled best point is sufficiently close to the lower bound given by the Lipschitz constant. I have added an implementation of an SP solver in crates/argmin/src/solver/shubertpiyavskii/, modifying crates/argmin/src/lib.rs and crates/argmin/src/solver/mod.rs accordingly. Additionally, I have added an example run of the solver to examples/shubertpiyavskii/.
4bf9bf2 to
5d75b8d
Compare
|
One last fix; code and doctests were all working, but clippy got mad at something non-idiomatic in a match branch, hehe. Now it should be 100% ready, pending your approval @stefan-k :) (No rush!) |
|
Hi @stefan-k! Definitely no rush, but I just wanted to ask if you still planned to get around to reviewing this at some point in the future (doesn't matter when!) 🙂 I also plan to make a PR sometime soon for Differential Evolution as per my conversation with @steinhauserc in #499, if you think that's a good idea. |
Sincere apologies, I totally missed this! Thanks for your contribution, I'll try to do a review within the coming days (hopefully even today). Please ping me again should I not get back to you by Sunday.
Absolutely yes! :) |
|
|
||
| impl<F: ArgminFloat> PartialEq for SearchInterval<F> { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.lower_bound == other.lower_bound |
There was a problem hiding this comment.
This compares two floats for equivalence which is probably not what you want here. I assume you want to check if those two values are close to each other within a given tolerance. I would avoid hiding this operation within an PartialEq implementation, instead make it explicit in the code (and potentially even make the tolerance configurable with a sane default).
There was a problem hiding this comment.
This compares two floats for equivalence which is probably not what you want here. I assume you want to check if those two values are close to each other within a given tolerance. I would avoid hiding this operation within an
PartialEqimplementation, instead make it explicit in the code (and potentially even make the tolerance configurable with a sane default).
I implemented this to facilitate the Ord/PartialOrdimpls for SearchInterval (as I described in another review response). On this particular issue, I'm more inclined to agree with you and extract the logic into the Ord/PartialOrd impls themselves?
Given my explanation for the Ord/PartialOrd impls, what do you think is most appropriate?
| impl<F: ArgminFloat> Ord for SearchInterval<F> { | ||
| fn cmp(&self, other: &Self) -> Ordering { | ||
| other.lower_bound.partial_cmp(&self.lower_bound).unwrap() | ||
| } | ||
| } | ||
|
|
||
| impl<F: ArgminFloat> PartialOrd for SearchInterval<F> { | ||
| fn partial_cmp(&self, other: &Self) -> Option<Ordering> { | ||
| Some(self.cmp(other)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you explain your reasoning behind implementing Ord/PartialOrd for SearchInterval? Thanks!
There was a problem hiding this comment.
Could you explain your reasoning behind implementing Ord/PartialOrd for SearchInterval? Thanks!
Hi @stefan-k - I did this so that we can use (for some F: ArgminFloat the BinaryHeap<SearchInterval<F>> needed to process the search intervals by those with smaller lower bounds first. To my understanding, this is the easiest way to implement a priority queue for the search intervals, but definitely let me know if you have a better implementation alternative?
|
Hi, @stefan-k—I'll try to respond to your review over the weekend :) |
|
Hi @stefan-k, sorry for the delay. I'll look over and respond to your comments later today for sure! |
|
@stefan-k, I just responded :) TL;DR: The |
|
Hi @stefan-k, just following up again. |
|
Hi @stefan-k, any further changes needed? 🙂 |
|
Hi @stefan-k, just following up again! :) |
The Shubert-Piyavskii method optimizes a univariate, Lipschitz continuous function inside a specified interval. Given an accurate Lipschitz constant, it is guaranteed to come within an arbitrary epsilon of the true global minimum. Unlike most other deterministic global optimizations algorithm (e.g., golden-section search), it is not restricted to unimodal functions; moreover, it does not require an initial guess. It deterministically samples points from ever-narrowing subintervals of the search space until the sampled best point is sufficiently close to the lower bound given by the Lipschitz constant.
I have added an implementation of an SP solver in crates/argmin/src/solver/shubertpiyavskii/, modifying crates/argmin/src/lib.rs and crates/argmin/src/solver/mod.rs accordingly. Additionally, I have added an example run of the solver to examples/shubertpiyavskii/.