search

BlueBrain / snap

The Blue Brain Pythonic Simulation and Network Analysis Productivity layer
https://bluebrainsnap.readthedocs.io/
GNU Lesser General Public License v3.0
19 stars 10 forks source link

add kwarg to `firing_rate_histogram` to allow considering gids outside the report #263

Closed joni-herttuainen closed 4 months ago

joni-herttuainen commented 9 months ago

firing_rate_histogram is limited to only consider gids within the report (see: here).

bluepy did not have the same behavior in it's implementation of the plot. Therefore, to ease the comparison to simulation runs done in bluepy, it was suggested to add a keyword argument to have similar behavior.

Another motivation is to get an overview of the entire population firing behaviour / firing rates.

mgeplf commented 9 months ago

I remember there being a lot of back and forth about this, and how having arguments like this will blow up over time as everyone has their own desires. In light of that, can you check if there isn't another way to include all gids, instead of just the ones in the report? I don't see one, but it's worth looking. Otherwise, adding an argument makes sense to me; perhaps it can be a function that is passed in that filters desired GIDs, with the default function being the current behavior?

joni-herttuainen commented 9 months ago

Yeah, I remember Thomas not being happy about the plots being part of the package in the first place. Reasoning being that it's downright impossible to create a plot that works for everyone in every case and it will need to be tweaked much like here now.

I'll take a further look and see what can be done and then give a deep thought to "should it be done".

mgeplf commented 9 months ago

"should it be done".

Having plots, including firing_rate_histogram is required, but yeah, have a look on how best to be able to include gids outside of the report.

joni-herttuainen commented 9 months ago

Having plots, including firing_rate_histogram is required

Of course, and I would not suggest not having plots, it was merely in terms of how much customisation capabilities is it worth having in these functions.

Take, for example, firing_rate_histogram: simple function, roughly 20 LOC, and 1/4 of it is creating the plot instance and setting labels. What I'm thinking, is: should we try to add stuff into it for it to be more flexible, or should we suggest users copy-paste the code and change the relevant parts to suit their needs?

Anyway, I'll try to find the simplest solution to the issue at hand, and let's see where we stand after that.