-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add random selection operator #69
Conversation
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.
Left some implementation notes below
|
||
pub fn random<T: Chromosome, S: ChromosomeWrapper<T>>(population: &Vec<S>, count: usize) -> Vec<&S> { | ||
// We must use index API, as we want to return vector of references, not vector of actual items | ||
let indices = rand::seq::index::sample(&mut rand::thread_rng(), population.len(), count); |
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 line panics with quite good error message when count
> population.len()
pub fn random<T: Chromosome, S: ChromosomeWrapper<T>>(population: &Vec<S>, count: usize) -> Vec<&S> { | ||
// We must use index API, as we want to return vector of references, not vector of actual items | ||
let indices = rand::seq::index::sample(&mut rand::thread_rng(), population.len(), count); | ||
let mut selected: Vec<&S> = Vec::with_capacity(count); |
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.
We could really consider using custom, thread local memory allocator, so that we don't have to reach global memory pool every time such function is called.
## Description Should be merged only after * #69 is merged. Added missing selection operator. This is one of two popular implementations. Second one has additional `r` parameter, which is used to decide which of random selected individuals to use as a parent. I left the second implementation behind for now, as presence of additional parameter raises some problems with our current approach to operator implementations. Namely we can't just add additional param to function declaration as it would break existing selection operator API in GA implementation (how do we decide whether to pass 2 or 3 params to operator in runtime?). One possible solution might be to implement operators as traits (concrete implementations as structs with `FnMut` trait implemented). Such approach would not only allow for operator parameterization but also for some kind of function overload which is not present in Rust. ## Linked issues Closes #37 ## Important implementation details N/A
Description
Adding missing selection operator.
Linked issues
Resolves #36
Important implementation details
I left them below in form of a review.