Closed ntolley closed 5 months ago
@jasmainak @rythorpe perhaps it'd be a good idea to make this a separate file? If we can find a solution to updating the section colors very rapidly, we can even consider adding a "play" button inside the GUI (or on the HNN website!) like the example here: https://matplotlib.org/stable/gallery/animation/unchained.html
@chenghuzi you might have some good insight on how to make the current code more "reactive"
Attention: Patch coverage is 95.17544%
with 11 lines
in your changes are missing coverage. Please review.
Project coverage is 91.51%. Comparing base (
819f4f4
) to head (db60ec2
). Report is 1 commits behind head on master.:exclamation: Current head db60ec2 differs from pull request most recent head e9a94ef
Please upload reports for the commit e9a94ef to get more accurate results.
Files | Patch % | Lines |
---|---|---|
hnn_core/viz.py | 95.13% | 11 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Agnostic about saving to a different file ... see my comment about the azimuth/elevation control. Maybe it makes the class thinner?
Dropping this here for easy testing once we get closer to merging. I think rendering movies is going to be a longish wait for larger networks, but not absurdly so (maybe like 5-10 minutes? if you downsample?)
import hnn_core
import os.path as op
from hnn_core import jones_2009_model, simulate_dipole, read_params
from hnn_core.network_models import add_erp_drives_to_jones_model
from hnn_core.viz import NetworkPlot
hnn_core_root = op.dirname(hnn_core.__file__)
params_fname = op.join(hnn_core_root, 'param', 'default.json')
params = read_params(params_fname)
params.update({'N_pyr_x': 3, 'N_pyr_y': 3})
net = jones_2009_model(params)
net.set_cell_positions(inplane_distance=300)
add_erp_drives_to_jones_model(net)
dpl = simulate_dipole(net, dt=0.5, tstop=170, record_vsec='all')
net_plot = NetworkPlot(net)
net_plot.export_movie('demo.gif', dpi=200)
@jasmainak @rythorpe perhaps it'd be a good idea to make this a separate file? If we can find a solution to updating the section colors very rapidly, we can even consider adding a "play" button inside the GUI (or on the HNN website!) like the example here: https://matplotlib.org/stable/gallery/animation/unchained.html
@chenghuzi you might have some good insight on how to make the current code more "reactive"
Great work! Although, I'm concerned about the reactivity of our GUI. We are taking snapshots from the buffer and using imshow
to render network images for now. So, it might be difficult to have real-time interaction in the new GUI with these new settings. But this will not affect the current function so there is nothing else to worry about.
@rythorpe @jasmainak this PR is ready for review! Two weird errors have come up however. First it seems like sphinx-gallery doesn't support python versions older than 3.8
Does this mean our minimum python version needs to be updated?
Also the windows unit tests are failing with an error in the GUI code...
This is the offending line in case you guys have any ideas on the possible cause: https://github.com/ntolley/hnn-core/blob/e3aa27c91ba4ed7478d2214e53fea618b793f45b/hnn_core/tests/test_viz.py#L23C5-L23C5
Yup just confirmed, sphinx now requires python>=3.8
@rythorpe @jasmainak this PR is ready for review! Two weird errors have come up however. First it seems like sphinx-gallery doesn't support python versions older than 3.8
Does this mean our minimum python version needs to be updated?
Also the windows unit tests are failing with an error in the GUI code...
This is the offending line in case you guys have any ideas on the possible cause: https://github.com/ntolley/hnn-core/blob/e3aa27c91ba4ed7478d2214e53fea618b793f45b/hnn_core/tests/test_viz.py#L23C5-L23C5
Ugh, I'm guessing the most recent version of matplotlib has diverged for windows vs. linux. Something to do with the agg
backend....
Ok updating the python version used for CircleCI worked, however it seems something in the MNE code is breaking the somato example now:
it seems something in the MNE code is breaking the somato example now:
looks like nibabel
is now being used in MNE for reading the surface files ... I reported this upstream:
https://github.com/mne-tools/mne-python/pull/11565/files#r1337642633
hopefully it'll get fixed soon but probably still won't be reflected in the current release. We may want to add nibabel
as an added install in the CI with a note to remove it when MNE does it automatically
@rythorpe @gtdang could you guys review and merge this PR if it's good by you? I am finding it difficult to find time and don't want to block progress ... this could be a good excuse for a release.
Just in case this slipped your attention @ntolley, this PR is sooooo close ;-)
@ntolley do you know if plot_cell_morphology
sets the length of each cell section before plotting it? Or are the section endpoints already updated with the appropriate length? Apologies if we already hashed this out, I just can't remember and was wondering about this the other day....
@rythorpe they are updated automatically! #433 fixed this early on, but then Rajat's project made a new and improved version where he implemented the Neuron functions for calculating end points directly in our python code (necessary for saving this infromation in a JSON/dict format)
@jasmainak this is ready for a final round of reviews when you have the chance!
I think there are some tasks that might be delegated to followup PR's (e.g. a better jupyter notebook tutorial?), but in terms of basic functionality and readability I think this is in a good place
@ntolley this is tremendous!!! 🥳 when can I have the privilege of merging it? I tried it and it works really nice. @rythorpe did you want to look? It's good to go on my end.
One feature request: perhaps we can have a small text in the corner showing the timestamp ... I was a bit confused whether the plot was still updating (or is it instantaneous?) when I moved it around.
@gtdang do you still have remaining objections? Merging seems blocked because of your comments
Don't forget to update whats_new.rst
!
Aside from @jasmainak's spelling error and my incredibly nitpicky punctuation suggestion above, this looks great @ntolley !
I've enabled auto-merge! Thanks @ntolley for this incredible feature - I'm very excited to put it to good use :smile:
Here's my first pass at creating a viz class that can be used to quickly render the full 3D network, as well as membrane potentials of individual sections. This is meant to be largely generic and flexible so that it can be co-opted for other purposes. I think the most interesting applications at the moment would be:
The current strategy for the class is to do all of the heavy lifting at the initialization (namely creating the 3D object, and mapping the voltage recordings to the appropriate colors). For a quick demo, the code below can be run on my laptop on a single core in approximately 20 sec:
If you want to play around with visualizing the voltages, you can try the following code block: