Skip to content

Fix neuron, training, and ODE bugs and tighten dependencies#818

Merged
chaoming0625 merged 4 commits intomasterfrom
update
Mar 11, 2026
Merged

Fix neuron, training, and ODE bugs and tighten dependencies#818
chaoming0625 merged 4 commits intomasterfrom
update

Conversation

@chaoming0625
Copy link
Member

@chaoming0625 chaoming0625 commented Mar 11, 2026

Summary by Sourcery

Improve error handling, state updates, numerical methods, and dependency configuration across neuron dynamics, training, math utilities, and build metadata.

Bug Fixes:

  • Provide descriptive ValueError messages for invalid spike reset modes in LIF neuron updates.
  • Handle zero and negative slice steps correctly when converting slices to counts in dynamical systems.
  • Raise the correct NoLongerSupportError when an unsupported seed is provided in back propagation training.
  • Compute scalar metrics using NumPy means instead of JAX conversions in training loops to avoid backend issues.
  • Update synapse state variables by writing to underlying .value attributes instead of relying on in-place ops on wrappers.
  • Fix NodeDict and VarDict update logic when given (key, value) tuples and improve duplicate-key error messages to reference existing objects.
  • Correct batch dimension slicing logic in variable size_without_batch to use the configured batch axis.
  • Ensure load_state reports missing nodes when keys are absent from the provided state dict.
  • Narrow overly broad exception handlers in convolution layers and Flax interoperation to specific import/shape errors.
  • Correct Bogacki–Shampine integrator coefficients and default ODE method assignment in ODE integrator configuration.
  • Fix Array.value setter to use the stored value directly instead of invoking tracer checks that can break assignment.

Enhancements:

  • Tighten Python version and library dependency requirements in pyproject and requirements to reflect supported environments and minimum versions.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 11, 2026

Reviewer's Guide

This PR improves error reporting and robustness across multiple modules, fixes several bugs in state handling and object updates, tightens exception handling, and updates dependency/version requirements.

File-Level Changes

Change Details Files
Clarify spk_reset error handling in LIF neuron update paths.
  • Replace bare ValueError raises with descriptive error messages indicating unknown spk_reset mode and allowed values in multiple LIF.update branches, both gradient and no-gradient code paths.
brainpy/dyn/neurons/lif.py
Make slice length computation robust to zero and negative steps.
  • Disallow zero step in _slice_to_num by raising ValueError.
  • Split the counting loop into separate branches for positive and negative step slices to compute element count correctly.
brainpy/dynsys.py
Fix BackPropagation training behavior and metric computation.
  • Change seed handling to actually raise NoLongerSupportError instead of just instantiating it.
  • Replace JAX-based mean computation on epoch metrics with NumPy-based np.mean over np.asarray(v) for both train and test metrics.
brainpy/train/back_propagation.py
Correct synapse state updates to operate on underlying variable values.
  • Change updates of h and x in abstract synapse models to mutate .value explicitly (e.g., self.h.value = self.h.value + self.a * x, self.x.value = self.x.value + pre_spike) instead of relying on in-place += which may not affect the wrapped variable.
brainpy/dyn/synapses/abstract_models.py
Fix NodeDict/VarDict update semantics and duplicate-key detection.
  • When updating from a (key, value) tuple, assign arg[1] instead of the entire args[1] sequence in both NodeDict.update and VarDict.update.
  • Improve duplicate key error in NodeDict.setitem to only raise when an existing value with a different identity is present, and to reference the existing object in the error message.
brainpy/math/object_transform/base.py
brainpy/math/object_transform/variables.py
Improve load_state robustness when some nodes are missing in the provided state_dict.
  • Before calling node.load_state, check if the node name exists in state_dict; if not, record it in missing_keys and skip loading for that node.
brainpy/helpers.py
Tighten exception handling around optional dependencies and masking in conv layers.
  • In Flax interop, restrict import error handling to ImportError and ModuleNotFoundError instead of a bare except.
  • In Conv and ConvTranspose, replace bare except around lax.broadcast_shapes with catching ValueError and TypeError, and keep raising a clear ValueError when shapes mismatch.
brainpy/dnn/interoperation_flax.py
brainpy/dnn/conv.py
Update and tighten library and Python version requirements.
  • Require numpy>=1.15 and bump minimum versions for braintools and brainpy_state in requirements.txt.
  • Raise minimum Python version from 3.10 to 3.11 in pyproject.toml.
  • Constrain brainunit dependency to brainunit>=0.2.0 and align braintools/brainpy_state versions in pyproject.toml.
requirements.txt
pyproject.toml
Fix numerical integrator configuration and coefficients.
  • Correct Bogacki–Shampine tableau coefficient from '4/0' to '4/9'.
  • Fix set_default_odeint so it assigns the chosen method to _DEFAULT_ODE_METHOD instead of incorrectly overwriting _DEFAULT_DDE_METHOD.
brainpy/integrators/ode/adaptive_rk.py
brainpy/integrators/ode/generic.py
Adjust ndarray Array behavior to avoid tracer checks on value assignment.
  • Change Array.value setter to use self._value directly instead of calling _check_tracer(), altering how assignments interact with potential tracer logic.
brainpy/math/ndarray.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@chaoming0625
Copy link
Member Author

@sourcery-ai title

@github-actions github-actions bot added dependencies Pull requests that update a dependency file build labels Mar 11, 2026
@sourcery-ai sourcery-ai bot changed the title Update Fix neuron, training, and ODE bugs and tighten dependencies Mar 11, 2026
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In set_default_odeint() you’re assigning to _DEFAULT_DDE_METHOD instead of _DEFAULT_ODE_METHOD, which will leave the ODE default unchanged and unexpectedly overwrite the DDE default.
  • The change in Array.value’s setter from self._check_tracer() to self._value bypasses the tracer check; if _check_tracer() enforces important invariants (e.g., JAX tracing or immutability constraints), this could silently break those assumptions and should be reconsidered or replicated explicitly.
  • In BackPropagation.fit, replacing jnp.mean(bm.as_jax(bm.asarray(v))) with np.mean(np.asarray(v)) forces metrics computation onto the host/CPU and may introduce unnecessary device-host transfers; consider whether keeping this on the JAX backend (or via bm.mean) would better match the rest of the code’s execution model.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `set_default_odeint()` you’re assigning to `_DEFAULT_DDE_METHOD` instead of `_DEFAULT_ODE_METHOD`, which will leave the ODE default unchanged and unexpectedly overwrite the DDE default.
- The change in `Array.value`’s setter from `self._check_tracer()` to `self._value` bypasses the tracer check; if `_check_tracer()` enforces important invariants (e.g., JAX tracing or immutability constraints), this could silently break those assumptions and should be reconsidered or replicated explicitly.
- In `BackPropagation.fit`, replacing `jnp.mean(bm.as_jax(bm.asarray(v)))` with `np.mean(np.asarray(v))` forces metrics computation onto the host/CPU and may introduce unnecessary device-host transfers; consider whether keeping this on the JAX backend (or via `bm.mean`) would better match the rest of the code’s execution model.

## Individual Comments

### Comment 1
<location path="brainpy/integrators/ode/generic.py" line_range="137" />
<code_context>

     global _DEFAULT_DDE_METHOD
-    _DEFAULT_ODE_METHOD = method
+    _DEFAULT_DDE_METHOD = method


</code_context>
<issue_to_address>
**issue (bug_risk):** Default ODE integrator is no longer updated; code now writes to `_DEFAULT_DDE_METHOD` twice instead of `_DEFAULT_ODE_METHOD`.

This line should continue assigning to `_DEFAULT_ODE_METHOD`; `_DEFAULT_DDE_METHOD` is already set earlier in the function.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.


global _DEFAULT_DDE_METHOD
_DEFAULT_ODE_METHOD = method
_DEFAULT_DDE_METHOD = method
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Default ODE integrator is no longer updated; code now writes to _DEFAULT_DDE_METHOD twice instead of _DEFAULT_ODE_METHOD.

This line should continue assigning to _DEFAULT_ODE_METHOD; _DEFAULT_DDE_METHOD is already set earlier in the function.

@chaoming0625 chaoming0625 merged commit f917281 into master Mar 11, 2026
19 checks passed
@chaoming0625 chaoming0625 deleted the update branch March 11, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant