fix(examples): repair 6 broken example scripts (API drift, data path, channels)#858
Merged
Conversation
… channels)
Triaged all 24 examples/ scripts. 14 already pass; the following 6 are fixed
and verified (construction + forward pass / short sim, or full run):
- integrate_flax_into_brainpy: bp.layers.GRUCell -> bp.dyn.GRUCell (relocated)
- whole_brain_simulation_with_fhn / _sl_oscillator: load the bundled
hcp.npz from the brainpy package (the old ../../tests/simulation/data
path no longer exists; the file lives at brainpy/dyn/rates/data/hcp.npz)
- Song_2016_EI_RNN / highdim_RNN_Analysis: RandomState(seed=...) ->
positional (the kwarg was renamed to seed_or_key)
- mnist_ResNet: parameterize ResNet in_channels and pass 1 for 1-channel
MNIST (was hardcoded to 3, raising a channel-mismatch ValueError)
The remaining 4 are unchanged: they need optional deps or exceed the sandbox
time budget -- reservoir-mnist (functional but ~11 min of training),
integrate_brainpy_into_flax-{lif,convlstm} (tensorflow_datasets),
spikebased_bp_for_cifar10 (torch).
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRepairs six example scripts so they run against the current BrainPy API and bundled data, including fixing a removed GRUCell symbol, updating HCP connectome data loading to use the installed package path, adjusting RandomState construction to the new API, and parameterizing the ResNet input channels so the MNIST example works with 1-channel images. Flow diagram for loading bundled HCP connectome dataflowchart LR
A[Network.__init__] --> B[import brainpy as bp]
B --> C[get dirname of bp.__file__]
C --> D[join path dyn/rates/data/hcp.npz]
D --> E["np.load(hcp_path)"]
E --> F["bm.asarray(hcp['Cmat'])"]
E --> G["bm.round(hcp['Dmat'] / signal_speed / bm.dt)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The HCP data path resolution in the two whole-brain examples hardcodes the package layout via
os.path.dirname(bp.__file__); consider using a resource-loading helper (e.g.,importlib.resourcesor a BrainPy-provided API) so the examples remain robust to internal package reorganization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The HCP data path resolution in the two whole-brain examples hardcodes the package layout via `os.path.dirname(bp.__file__)`; consider using a resource-loading helper (e.g., `importlib.resources` or a BrainPy-provided API) so the examples remain robust to internal package reorganization.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ran a crash-safe, strictly-sequential triage of all 24
examples/scripts(isolated subprocess each,
ulimit -vcapped, thread-capped, headlessAgg,OS timeout,
PYTHONPATH=repo so imports resolve to this source). Result:14 already pass, 6 fixed here, 4 unchanged (need optional deps or
exceed the time budget).
Fixes (all verified green)
integrate_flax_into_brainpybp.layers.GRUCellremovedbp.dyn.GRUCellwhole_brain_simulation_with_fhn../../tests/simulation/data/hcp.npzpathbrainpy/dyn/rates/data/hcp.npzvia package dirwhole_brain_simulation_with_sl_oscillatorhcp.npzpathSong_2016_EI_RNNRandomState(seed=...)kwarg renamedRandomState(seed)highdim_RNN_AnalysisRandomState(seed=...)mnist_ResNetin_channels=3vs 1-channel MNIST →ValueErrorin_channels(default 3), pass1for MNISTVerification: heavy scripts smoke-tested (network construction + one forward
pass / 20 ms sim);
Song_2016_EI_RNN(95% acc) andhighdim_RNN_Analysis(fixed-point search) run to completion.
Unchanged (documented, not bugs)
reservoir-mnist— functional but ~11 min of training (exceeds the 10 min sandbox timeout).integrate_brainpy_into_flax-lif,integrate_brainpy_into_flax-convlstm— requiretensorflow_datasets.spikebased_bp_for_cifar10— requirestorch.Summary by Sourcery
Update several example scripts to align with current BrainPy APIs, bundled data locations, and dataset channel configurations.
Bug Fixes: