Particle Swarm Optimization refactor #68
Conversation
…single run of the algorithm, added comment description of config parameters
…o pso probes to move all logging logic into them. Removed log_interval from PSOAlgorithm's parameters.
There was a problem hiding this comment.
Good job 🎉
I left some notes below but generally it can be shipped.
Moreover I want to discuss following topics:
Probe implementation
I really like your idea to use composition to separate logging policy (time based, iteration based, etc.) from logging format (csv, json, etc.). I believe this it the way to go and maybe we should consider moving this code from pso module to some common / shared or something like that, because other algorithms could also make use of that.
The problem I have is that we are storing information from each generation in RAM and this could lead to some big issues with memory efficiency. Imagine that some computation is running for couple of days with logger granularity of 10 generations => I'm not sure, but I can see it meaning few GBs of RAM being occupied only by logging system. I can also see why it is bad idea to flush data to file every single time we get log request -- this could lead to massive performance penalties. Generally I think that we should go for some kind of middle ground like batching. We could store some arbitrary number of log records in buffer and dump it to disk only after buffer is full.
This comment applies to whole logging system and concerns every algorithm.
I would also prefer to create separate issue (task) for this and discuss the topic further there, rather than introducing more changes in this PR as it already is big and touches more than one thing (PSO, probes, examples...).
| use crate::ga::*; | ||
| use crate::pso::*; | ||
| use crate::ff::*; | ||
| use crate::ff::auxiliary::*; | ||
| use crate::ff::probe::console_probe::ConsoleProbe; | ||
| use crate::pso::builder::PSOAlgorithmBuilder; |
There was a problem hiding this comment.
It's completely fine for now, but I'm leaving this comment to remind us that we should discuss naming conventions. E.g. whether we should name it pso::builder::PSOAlgorithmBuilder or pso::builder::Builder. Same goes for calling our algorithm's implementations literally "Algorithm": "PSOAlgorithm" instead of just "PSO". The same considerations apply to every other algorithm. I will consider starting a discussion on this topic.
| for i in 0..self.particles.len() { | ||
| self.particles[i].update_velocity(&self.best_position, inertia_weight, cognitive_coefficient, social_coefficient); | ||
| } | ||
| self.particles.par_iter_mut() |
There was a problem hiding this comment.
I believe we can leave it for now, but I think we should left parallelization out until we have some benchmarking system implemented. Moreover we should discuss introduction of parallelism with our client, to discuss whether there are any expectations.
| } | ||
|
|
||
| impl CsvProbe { | ||
| pub fn new(generations: usize) -> CsvProbe { | ||
| pub fn new(filename: &'static str) -> CsvProbe { |
There was a problem hiding this comment.
I guess it would be fine to take ownership here, but it is up to you.
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
Description
Done a big refactor of PSO Algorithm, including better consistency with other algorithms, better logging through probes, parallelisation (simple, but still) and code comments with explanations and examples.
Linked issues
Possibly closes #10 and closes #13
Important implementation details
I'm quite happy with how Probes for PSO turned out - I believe they offer quite a lot of flexibility in terms of logging algorithm's results.