-
Notifications
You must be signed in to change notification settings - Fork 93
Fix for Bug in Powell's Method, issue #51 #78
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
base: main
Are you sure you want to change the base?
Fix for Bug in Powell's Method, issue #51 #78
Conversation
After carefully looking at the test failure, it seems the test failure came from setting the "random" in HillClimb initialize here to zero. It looks like there is an expectation of what should be set here, which afected the SimulatedAnnealingOptimizer expectations. |
@smilingprogrammer |
self.setup_line_search() | ||
|
||
def update_search_directions(self): | ||
if self.initial_position is None or not self.positions_valid: |
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.
This should not be here, because it affects the entire method (so put it to the method call) and is probably redundant to if self.cycle_count > 0 and self.positions_valid
.
if self.current_search_dim >= len(self.search_directions): | ||
self.current_search_dim = 0 | ||
self.cycle_count += 1 | ||
if self.cycle_count > 0 and self.positions_valid: |
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.
pos_new = self.hill_climb.conv.position2value(pos_new) | ||
pos_new = np.array(pos_new) | ||
next_idx = self.hill_climb.iterate() | ||
if next_idx is None: |
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.
How can this be triggered? When is next_idx ever "None"?
if next_idx is None: | ||
next_idx = self.hill_climb.init_pos() | ||
|
||
pos_new = np.array(self.hill_climb.conv.position2value(next_idx)) |
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.
In later iterations next_idx and pos_new are equal. Does Powell's Direction Method fall back to its line-search algorithm (in this case hill-climbing) in some cases? Because it seems that way.
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.
Yes, in some cases. If it doesn't find a better neighbor, it can return thesame position again.
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.
Please add some explanations to the new methods. It would also be better if your methods have some arguments, that make sense. Don't just use self for everything.
How is the angle for the new line-search calculated? I do not see how a search-space is constructed, that is directed to a specific angle. It all still looks 90-degress to me.
Hi @SimonBlanke Thanks for the feedback, tto move forward, can you provide me with the code you used for the testing above, which displays how it works in real time. And also what the simulation (above graph gif) expected to look like after making corrections. I think this will help a lot, to get clarity on what the expectation should be. |
The module I created the plot with can be found here: But beware, that I just use this privately for my work. At the moment this is not documented. An example of the API would look like this: spg = SearchPathGif(path=".")
spg.add_optimizer(
optimizer=optimizer_class,
opt_para={},
initialize=initialize,
n_iter=n_iter,
random_state=random_state,
)
spg.add_test_function(
objective_function=objective_function,
search_space=search_space,
constraints=constraints_list,
)
spg.add_plot_layout(
name="my_plot",
title=True,
)
spg.create()
Algorithm from your PR: 90 degree angles and later normal hill-climbing Algorithm as discussed: It should allow a continuous range of angles. Please reread explanations of powell's direction method to understand the goal. Unfortunately I could not find the official paper for this. This presentation seems to offer some easy to follow explanations and plots: |
Thank you for the detailed reply. |
Hi, sorry i have been away for a while. Looking forward to your response. |
You do not have to open a new PR. You can add your corrections to this PR if you like. Is the algorithm behaviour in this graph what you would expect from the powell's direction method? |
Yes the graph direction behaviour was what i got from the new powell's method that i included in the gist link. |
yes I see that. My question was about the way powell's direction method should work (in theory/from literature) vs how your implementation of powell's direction method works according to the search-path-gif you provided. It is difficult to see how this represents the algorithm. |
What should be the expectation here? Will be better if there is a description of what the expectation should be. The Powell's method has a really low resource on it that is available online. The pdf you shared might even be the most informative i have come accross for this method. |
I have found the paper!!! Will take my time to go through it. |
Yes, you can do that. But it is not a requirement. The main focus is on fixing the limited angle when changing direction. |
Motivation
This PR solves the bug in the current implementation of the Powell's Method, where the change in direction only happens orthogonally (or 90°) to the previous direction. Issue #51 .
Description of the changes
We now track the cycle start and end positions, by storing the best‑found position at the beginning of each n‑direction cycle in
last_best_position
. After completing all n one‑dimensional searches, it records the new best position incurrent_best_position
.It now computes displacement vector by calculating the
displacement = current_best_position – last_best_position
and normalize it to unit length. This now represents the “true” sweeping direction of the search over the last cycle.It drops the oldest direction and append
new_direction
, allowing any angle in [0°, 180°] instead of only the coordinate axes.Now in
setup_line_search()
, once the Hill-climb is instantiated, it immediately sets the Hill-climb’sto the grid‐index of the current global best so that every line search truly starts from the optimum, rather than from a random neighbor.
Removed the branch that used
init_pos()
for the first few sub‑iterations; now every sub‑step refines from the known best along that line.Now through continuous‑angle updates which happens in
update_search_directions()
method, the optimizer measures how far it has moved during each complete cycle and then scales that movement to a unit vector. This unit vector replaces the oldest search direction, allowing the algorithm to adopt any angle between 0° and 180°. With this, It lets the optimizer follow the function’s actual main direction, instead of only moving in right‑angle turns.