-
Notifications
You must be signed in to change notification settings - Fork 334
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
Add BoundaryHandler and VelocityHandler #238
Conversation
Reference: #198 Inheriting from a metaclass, rather than an object, grants us more flexibility and stronger typechecking for our abstract/base classes. It's also the more Pythonic option. I also added `@abs.abstractmethod` decorators for abstract methods. Signed-off-by: ljvmiranda921 <[email protected]>
Removed the `environments` module in favor of the `plotters` module. Signed-off by: ljvmiranda921 <[email protected]>
Resolves: #201 Added fmt: off and fmt: on commments to n-dimensional arrays in the test suite to prevent the ugly wrapping of arrays. Ran the black package to format the whole project.
Resolves #209 This commit adds a reporter module that implements a Reporter class. Its goal is to separate the concern of logging and printing from the optimizer classes. Signed-off-by: Lester James V. Miranda <[email protected]>
Resolves #228 Added a cost function decorator which allows the user to write a cost function that serves as a model for how the cost will be computed for every one particle. The decorator is tested with a pytest file where the shape and the equality with an example function is checked. I added a note to the documentation that some numpy functions will return arrays with single values in them.
This commit exposes the "limits" parameter in the README, and updates some undocumented parameters in the docstrings. Signed-off-by: Lester James V. Miranda <[email protected]>
Created the BoundHandler class and implemented the "nearest" strategy.
Added a whole lot of documentation and implemented the Random strategy for the position handling.
I think we should put the Resample and the Shrink strategy into the |
Added RST files.
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 think the API looks nice and is similar to how I imagined it. My only comment is that we lessen "side-effects" in the handlers: just let them return the new position, do not update self.position
.
Perhaps we can pass position
, bounds
, *args
, and **kwargs
to each of the handlers; this removes the need for self.position
and any other attributes. Might also make our API more generalizable given weird handlers.
I think we should put the Resample and the Shrink strategy into the VelocityHandler as they require, unlike the others, the velocity to fix the particles position.
But they still handle the position wrt bounds, right? They just need the velocity matrix, but they don't update it right? This would still be better in BoundaryHandler
, let's just require velocity as one of the *args
.
I'm just wondering if we need the |
I think the reason why the author adds a set of velocity handlers is that we also want to update the velocity matrix that caused these particles to be out-of-bounds. If they are all Unmodified, there's a chance that they'll go out-of-bounds again. Sure, we can call |
Hey @whzup , I can help you with this if you want |
Note: I'm going to make more or less trivial tests, only to see if the strategies do what they should do. This in combination with the test for exceeding particles should be enough for now. |
Now all the strategies return the particles into the bounds
Hey @whzup , this honestly seems to be a difficult problem hmmm... |
Fixed a bug in the topologies where all the topologie would calculate the global best position instead of the best positions of the neighbourhoods. Fixed the VonNeumann tests as well as the optimizers such that they return the position with the best cost. I had to change the parameters for the optimization in the test_objective_func_with_kwargs to satisfy the conditions. Adjusted the code to fit the refactoring of the code. Especially in the topology tests where the sytax was wrong. Fixed the test by just checking if either the best or the second best position is inside of the matrix of the best positions... Fixed the error with the class passing by adding parenthesis which seem to have disappeared Cleaned up the PR and skipped the tests which require a very good cost as some topologies do not converge fast enough. Resolves #250
For modern python, let's support py35 above Signed-off-by: Lester James V. Miranda <[email protected]>
@ljvmiranda921, What do you think, shall I try to make some tests by calculating the desired positions by hand according to the algorithms or are the current tests sufficient? |
Hi @whzup , I think the current tests for now are sufficient. If this is good then just ping me and we can merge this to |
Fixed some spelling mistake and added the description for the reflective strategy.
Merged! Thank you for your great contribution, @whzup ! |
Description
This PR implements the new
BoundaryHandler
andVelocityHandler
classes that deal with the issue of particles that don't move in optimisations with boundary conditions. This is achieved by using different strategies that have been described in Hel10 (P. 28). These strategies include:There is one more:
I won't include the last one in this PR as it is probably provided by @jolayfield. After they've made their PR I'm going to embed their code in the
BoundaryHandler
class.The strategies for the
VelocityHandler
include:Related Issue
Resolves: #237
See also: #150, #234
Motivation and Context
In #234 we encountered a weird phenomenon when optimizing with boundary conditions. A lot of particles stopped moving while the optimization was still running. This is caused by the waiving of updating the position in the
compute_position()
function when it leaves the boundaries. To fix this issue we introduce a new class calledBoundaryHandler
as well as aVelocityHandler
which are going to reposition the particle instead of not updating it.How Has This Been Tested?
Types of changes
Checklist:
TODO:
BoundaryHandler
BoundaryHandler
strategiesBoundaryHandler
into theoperators
VelocityHandler
VelocityHandler
strategiesVelocityHandler
into theoperators