[MRG] Optimize rhythmic drives

[carolinafernandezp]

The goals are:

1. To be able to optimize parameters associated with rhythmic drives. The user sets the frequency bands and relative bandpower. In the example below, we wish to optimize power in the 9-11 Hz and 19-21 Hz bands in a ratio of 1 to 2 (i.e. twice as much power in the beta band as in the alpha band).

optim = Optimizer(net, tstop=tstop, constraints=constraints,
                  set_params=set_params, obj_fun='maximize_psd')
optim.fit(f_bands=[(9, 11), (19, 21)], relative_bandpower=(1, 2), scale_factor=scale_factor,
                  smooth_window_len=smooth_window_len)

Example output

Reversing the ratios

optim.fit(f_bands=[(9, 11), (19, 21)], relative_bandpower=(2, 1), scale_factor=scale_factor,
                  smooth_window_len=smooth_window_len)

2. To allow the user to define obj_fun as a callable.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 80.72289% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 92.55%. Comparing base (54ddd40) to head (78ab67d).
Report is 49 commits behind head on master.

❗ Current head 78ab67d differs from pull request most recent head d046764. Consider uploading reports for the commit d046764 to get more accurate results

Files	Patch %	Lines
hnn_core/gui/_viz_manager.py	59.09%	9 Missing ⚠️
hnn_core/optimization/objective_functions.py	83.33%	4 Missing ⚠️
hnn_core/optimization/general_optimization.py	76.92%	3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #673      +/-   ##
==========================================
- Coverage   92.66%   92.55%   -0.12%     
==========================================
  Files          27       27              
  Lines        4911     4945      +34     
==========================================
+ Hits         4551     4577      +26     
- Misses        360      368       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jasmainak]

can you share what the example outputs in the PR description?

[carolinafernandezp]

can you share what the example outputs in the PR description?

Updated! I'm not getting the expected ratios (1:2, α:β) so I'm looking into that now.

[rythorpe]

Per our previous discussion, is there a way to pull all parameters related to the objective function (e.g., scale_factor and smooth_window_len, target, etc.) out of the Optimizer class?

[carolinafernandezp]

I don't think we can take these out. For example, to optimize evoked drives, we use the scale_factor in _rmse_evoked (where we call simulate_dipole). If we don't let the user pass it in, the amplitude of the optimized dipole most likely will not match the amplitude of the target dipole. In the case of rhythmic drives, we don't really need to pass in target (so it is just an optional argument). Are you thinking we should pass these to the fit method instead? Or something completely different?

[jasmainak]

could you pass a "partial function" instead of passing each of the arguments?

https://docs.python.org/3/library/functools.html#functools.partial

[rythorpe]

The issue is that the arguments of the optimizer function/class should be general to any type of data simulated with hnn-core. For instance, smooth_window_len and scale_factor don't make sense if I want to optimize spike rates in the model. And it would be too chaotic to add similar sorts of preprocessing parameters to the API of this function for every conceivable type of objective function for every type of simulated data (spike timing, spike rates, dipole, LFP, CSD, membrane voltage/current, receptor conductance, etc.).

The selection and preprocessing of simulated data, in my mind, should be siloed within the objective function. My recommendation would be to allow the user to either define obj_fun as a callable, or pass in obj_fun_kwargs that apply to a given objective function defined with a string match. Advanced users can manipulate data preprocessing parameters by defining a partial function (as @jasmainak suggested), and less advanced users can rely on default parameters you define in the template objective functions or set them in obj_fun_kwargs as a catch-all for parameters that are specific to a given objective function.

[rythorpe]

This will also simplify your _run_opt functions and Optimizer.fit() methods considerably.

[carolinafernandezp]

Ok this makes sense, I just pushed a version that allows users to pass in obj_fun_kwargs and I'm working on allowing for a callable now.

[carolinafernandezp]

The user can now pass a callable for obj_fun. I'm testing this in test_general_optimization.py, test_user_obj_fun where obj_fun is set to maxime_csd which is just a function that maximizes CSD in a certain time range and for a certain electrode depth range.

[jasmainak]

whatever happened to this PR ? Was this close to merge? Is this something this year's GSoC student can work on?

[carolinafernandezp]

whatever happened to this PR ? Was this close to merge? Is this something this year's GSoC student can work on?

Hi, I'm going to work on getting this merged but I think it's definitely something this year's GSoC student can expand on.

[jasmainak]

@carolinafernandezp you seem to have rebase conflicts ... might be easiest to create a new branch from latest master, cherry pick your commits on to that branch, rename that branch to optimize_rhythmic and force push.

[jasmainak]

The build is failing because of the GUI example ... @gtdang @kmilo9999 how best to address this? maybe we could separate the GUI examples into a separate build using Github actions?

[gtdang]

We can just get rid of those GUI docs from the build. New docs for the GUI will just be markdown to reduce the complexity and fragility of the notebook builds.

[jasmainak]

How do we guarantee that the examples will run as expected for the user? unit tests?

[ntolley]

How do we guarantee that the examples will run as expected for the user? unit tests?

Exactly this, after going through the numerous problems with rendering the GUI tutorials, having hand-made tutorials with less extensive testing seems like the better option than having absolutely no tutorials due to an updated package

[jasmainak]

alright, go for it ... please remove it so we can move ahead with @carolinafernandezp 's PR

[ntolley]

@carolinafernandezp we've finally fixed the documentation issues, if you have the bandwidth in the next week or so would you want to do a quick zoom call to rebase this together and go ahead and merge it?

Also note for myself to close #176 once this is merged

[carolinafernandezp]

Sounds great @ntolley, how is Thursday the 18th? Any time would work for me

[ntolley]

Just tested this locally and everything is running great, thanks a bunch @carolinafernandezp this is a super useful contribution!!

[carolinafernandezp]

Thanks @ntolley and everyone that helped!👏🏻