[MRG] Optimization enhancements: CMA-ES algorithm and correlation loss function#1221
[MRG] Optimization enhancements: CMA-ES algorithm and correlation loss function#1221ntolley wants to merge 21 commits intojonescompneurolab:masterfrom
Conversation
| # "The current Network instance has external " | ||
| # + "drives, provide a Network object with no " | ||
| # + "external drives." | ||
| # ) |
There was a problem hiding this comment.
but aren't you then just adding on new drives with each iteration?
There was a problem hiding this comment.
In this instance, I found that it is easier to write a set_params function that directly modifies the properties of existing drives. Technically I write a workaround, but in any case I don't necessarily see why we should force users to make a function to either add brand new drives every time or modify existing ones.
| f"Joblib will run {n_trials} trial(s) in parallel by " | ||
| f"distributing trials over {self.n_jobs} jobs." | ||
| ) | ||
| if net._verbose: |
There was a problem hiding this comment.
A batch simulate test is failing that I'm at a loss to explain, it indicates that simulating serially is faster in parallel
The only real changes to batch simulate or parallel backends have to do with the verbose setting
Is it possible that these if blocks are slowing down parallel execution in a non-trivial way? But it doesn't make sense to me why serial would be faster...
| "bounds": constraints, | ||
| "tolfun": obj_fun_kwargs.get("tolfun", 0.01), | ||
| "maxiter": max_iter, | ||
| "popsize": obj_fun_kwargs.get("popsize", 100), |
There was a problem hiding this comment.
maybe the default value should be lower
| # I think that's also why you had to add the max(0.01, ...) for sigma in set_params_opt_drives | ||
| # _b_obj_func = cma.BoundDomainTransform(_obj_func, constraints) # evaluates fun only in the bounded domain | ||
|
|
||
| sigma = 1 / (np.array(constraints[1]) - np.array(constraints[0])) |
There was a problem hiding this comment.
@katduecker I actually forgot I already implemented a sigma that is scaled by the size of the bounds here!
| # I think that's also why you had to add the max(0.01, ...) for sigma in set_params_opt_drives | ||
| # _b_obj_func = cma.BoundDomainTransform(_obj_func, constraints) # evaluates fun only in the bounded domain | ||
|
|
||
| sigma = 0.25 * (np.array(constraints[1]) - np.array(constraints[0])) |
There was a problem hiding this comment.
@katduecker this is the right way to set it! the one earlier was wayyyy to small
the performance gain is quite impressive, you can use substantially smaller popsizes
This also removes ALL tests of the gen_opt.py file in order to continue investigating the failing test in test_batch_simulate.py
|
@asoplata the it was indeed the high |
|
Awesome, congrats! Yes I did my own secondary run of the same code here https://github.com/asoplata/hnn-core/actions/runs/22916219851 to test stochasticity and looks like you solved it! 😌 congrats! |
| return opt_params, obj, net_ | ||
|
|
||
|
|
||
| def add_opt_drives(net, tstop=200, n_prox=2, n_dist=1): |
There was a problem hiding this comment.
@asoplata @katduecker what do you think about removing these functions from the PR and including them in a textbook example in a different PR?
They're mostly convenience functions but as @katduecker brought up down below they need to be documented better
|
@katduecker @asoplata @carolinafernandezp this PR is ready to review! I'm thinking that we can add examples of use in a separate PR (perhaps on the textbook website?). The main goal here is just to make sure that the newly added Important notes on testing:
Related to making a future textbook example, it's unclear to me if |

This PR makes a lot of changes to implement new optimization functions, namely:
Rather than trying to merge this PR in one go, I think it should be broken into smaller tasks as this required a decent amount of changes all over the code base. However there is value to have a fully functioning branch to refer back to and play around with.