Skip to content
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

[MRG] Added BatchSimulate class for batch simulations #782

Merged
merged 3 commits into from
Jul 7, 2024

Conversation

samadpls
Copy link
Contributor

@samadpls samadpls commented Jun 3, 2024

Closes #140

hnn_core/batchsimulate.py Outdated Show resolved Hide resolved
@jasmainak
Copy link
Collaborator

Looks like a good start! Can you prepare an example so I can see the class in action?

@samadpls

This comment was marked as resolved.

@samadpls samadpls force-pushed the feat/batch_simulation branch from 670a65c to 8baa600 Compare June 3, 2024 14:53
@jasmainak
Copy link
Collaborator

I meant that you should add it to the examples gallery :)

@samadpls samadpls force-pushed the feat/batch_simulation branch 2 times, most recently from 150f201 to 54d65b4 Compare June 3, 2024 15:52
@jasmainak
Copy link
Collaborator

jasmainak commented Jun 5, 2024

As it stands ... the BatchSimulate does not really enable user to do more things by writing less code ... you need to brainstorm a bit how to make it more useful. Write the API first before starting to code.

batch = BatchSimulate(func)
dpls = batch.simulate()
batch.plot_dpl() # plot dipoles in a grid
batch.plot_psd()  # overlay multiple psds or plot in a grid

I'm just dreaming ... but think of ways people may want to use such an API and how to simplify boilerplate code for them

@ntolley
Copy link
Contributor

ntolley commented Jun 5, 2024

@jasmainak totally agree, I think the main utiluty of this function will be organizing code for saving/loading the potentially very large output files, along with some simulation metadata

To make this use less code, I think the best way will be to decide on a limited set of predefined functions. Perhaps even a class that helps users quickly specify the parameters they care about:

net = jones_2009_model()
func = make_evoked_simulator(net, weight_params='ampa', time_params='mean', custom_remove=['evprox1_L2Pyr_ampa'])
batch = BatchSimulate(func)
dpls = batch.simulate()

However, I want to avoid this make_simulator() scheme being too broad as it could quickly blow up in complexity and no be useful. The safest route right now will be to hard code the parameters for one of these functions, and have the example import the function.

Perhaps this will be a good way to replace this function too!

def add_erp_drives_to_jones_model(net, tstart=0.0):

hnn_core/batchsimulate.py Outdated Show resolved Hide resolved
hnn_core/batchsimulate.py Outdated Show resolved Hide resolved
hnn_core/batchsimulate.py Outdated Show resolved Hide resolved
hnn_core/batchsimulate.py Outdated Show resolved Hide resolved
@samadpls samadpls force-pushed the feat/batch_simulation branch 7 times, most recently from 957d0b8 to 724d001 Compare June 12, 2024 13:31
@ntolley
Copy link
Contributor

ntolley commented Jun 12, 2024

after discussion with @jasmainak, useful features will include

  • A callable that can be executed after each simulation, and calculates some sort of summary statistic
  • Plotting functions for uni-, bi-, and tri-variate parameter sweeps (including colormaps and subplot grids)

@samadpls samadpls force-pushed the feat/batch_simulation branch from 724d001 to 9a0b4ac Compare June 12, 2024 15:33
@samadpls samadpls force-pushed the feat/batch_simulation branch from 15d9cd4 to 00ad420 Compare June 12, 2024 17:19
hnn_core/batch_simulate.py Outdated Show resolved Hide resolved
hnn_core/batch_simulate.py Outdated Show resolved Hide resolved
@samadpls samadpls force-pushed the feat/batch_simulation branch 3 times, most recently from 1effef1 to 7ba6e2a Compare July 5, 2024 15:34
doc/api.rst Outdated Show resolved Hide resolved
@samadpls samadpls force-pushed the feat/batch_simulation branch 18 times, most recently from 9c8e262 to efa5de5 Compare July 5, 2024 18:36
@samadpls
Copy link
Contributor Author

samadpls commented Jul 5, 2024

@ntolley, It seems like the linkcheck passes now. I removed the import statement

from .batch_simulate import BatchSimulate

from the __init__.py file and the reference from api.rst.

@samadpls samadpls force-pushed the feat/batch_simulation branch from efa5de5 to f68b5d5 Compare July 5, 2024 18:59
@ntolley
Copy link
Contributor

ntolley commented Jul 7, 2024

Really strange! There's definitely a step I'm forgetting in terms of adding a new file/class for importing.

In any case really great start @samadpls, going to go ahead and merge and we can continue to improve on this in the next PR

@ntolley ntolley merged commit a91c3c3 into jonescompneurolab:master Jul 7, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add functionality for running batch simulations
5 participants