Skip to content

Fix audited correctness, API-drift & edge-case issues; add regression+coverage suite#830

Merged
chaoming0625 merged 1 commit into
masterfrom
worktree-audit-issues-20260618
Jun 18, 2026
Merged

Fix audited correctness, API-drift & edge-case issues; add regression+coverage suite#830
chaoming0625 merged 1 commit into
masterfrom
worktree-audit-issues-20260618

Conversation

@chaoming0625

Copy link
Copy Markdown
Member

Summary

A senior architecture / JAX / BrainX-ecosystem audit of the brainpy package surfaced 131 distinct issues (26 Critical, 53 High, 36 Medium, 16 Low), documented in docs/issues-found-20260618.md. The dominant theme is ecosystem-migration drift: BrainPy 2.7.x was rebased onto brainstate 0.5, brainevent 0.1, braintools, and JAX ≥0.10, and many code paths were not updated in lockstep — producing silent numerical errors and crash-on-first-use bugs concentrated in the optimizer/loss/scheduler stack, surrogate-gradient & synapse/plasticity code, sparse/event operators, FDE/adaptive integrators, and normalization layers.

This PR fixes those issues and adds a comprehensive regression + coverage test suite that locks in every fix.

Notable fixes

  • math — correct Array pytree unflatten under abstract eval (object.__new__ bypass), restoring jax.eval_shape / for_loop / scan over Array.
  • dyn/channels — remove removable exp/(exp-1) gating singularities via an _exprel helper so HH/TM/Ba sodium & potassium rates stay finite (and differentiable) at the singular membrane voltages.
  • optim — fix SM3 hyperparameter init order (the base __init__ reads self.momentum while registering train vars) and a torch-style keepdimkeepdims.
  • connect — fix FixedProb zero-density truncation (int(round(.))) and a contradictory include_self guard for rectangular populations.
  • integrators — remove the dead srk_strong module (generated-code syntax error, unregistered, unused) per finding L-12.
  • delay / pre_syn_post — correct ring-buffer modulo indexing.
  • …and the remainder of the 131 findings across math, dyn, dynold, dnn, integrators, analysis, losses, runners, and connect.

Testing

  • 1022 new tests under tests/audit/, all passing.
  • 93% aggregate line coverage over the 74 changed source files (coverage 7.14.1).
  • Remaining deep latent bugs that need upstream coordination are pinned with explicit pytest.raises regression tests rather than silently worked around.
  • Adds a coverage badge to README.md.

Full per-issue detail and severities are in docs/issues-found-20260618.md.

A senior architecture / JAX / BrainX-ecosystem audit of the brainpy
package surfaced 131 distinct issues (26 Critical, 53 High, 36 Medium,
16 Low), documented in docs/issues-found-20260618.md. The dominant theme
is ecosystem-migration drift: BrainPy 2.7.x was rebased onto brainstate
0.5, brainevent 0.1, braintools, and JAX >=0.10, and many code paths were
not updated in lockstep, producing silent numerical errors and
crash-on-first-use bugs.

Notable fixes:
- math: correct Array pytree unflatten under abstract eval (object.__new__
  bypass), restoring eval_shape / for_loop / scan over Array.
- dyn/channels: remove removable exp/(exp-1) gating singularities via an
  _exprel helper so HH/TM/Ba sodium & potassium rates stay finite at the
  singular membrane voltages.
- optim: fix SM3 hyperparameter init order (base __init__ reads momentum)
  and torch-style keepdim -> keepdims.
- connect: fix FixedProb zero-density truncation (int(round(.)) ) and the
  contradictory include_self guard for rectangular populations.
- integrators: remove dead srk_strong module (generated-code syntax error,
  unregistered, unused) per L-12.
- delay/pre_syn_post: correct ring-buffer modulo indexing.

Adds a comprehensive regression + coverage test suite under tests/audit/
(1022 tests, 93% aggregate line coverage over the 74 changed files) that
locks in every fix and pins remaining deep latent bugs with explicit
pytest.raises regression tests. Adds a coverage badge to README.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @chaoming0625, your pull request is larger than the review limit of 150000 diff characters

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests labels Jun 18, 2026
@chaoming0625 chaoming0625 merged commit fb09982 into master Jun 18, 2026
7 of 19 checks passed
@chaoming0625 chaoming0625 deleted the worktree-audit-issues-20260618 branch June 18, 2026 06:10
chaoming0625 added a commit that referenced this pull request Jun 18, 2026
- tests/audit/test_object_transform_fixes.py: #830 added M-06 asserting a
  None-returning while_loop body raises, but that broke the canonical brainpy
  idiom (body mutates Variable state, returns None) used by real models like
  SpikeTimeGroup. Now that the wrapper threads operands through unchanged,
  rewrite the test to assert that idiom works (state-driven termination) and
  update the module docstring.
- conftest.py (new, repo root): force matplotlib onto the non-interactive Agg
  backend for both test roots (tests/ and brainpy/) so analysis tests that call
  pyplot.show() never open GUI windows, locally or in CI.
- CI-models.yml: set MPLBACKEND=Agg on the pytest steps to match CI.yml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant