Skip to content

Refactor name mangling in NMODL ODE solver - #3543

Merged
nrnhines merged 32 commits into
masterfrom
jelic/fix_nmodl_name_mangling
Jan 23, 2026
Merged

Refactor name mangling in NMODL ODE solver#3543
nrnhines merged 32 commits into
masterfrom
jelic/fix_nmodl_name_mangling

Conversation

@JCGoran

@JCGoran JCGoran commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator

Needs #3544.

Fixes #3414.

NMODL's previous way of handling identifiers was somewhat convoluted; this PR introduces 2 functions, mangle_protected_identifiers, and its inverse, demangle_protected_identifiers, which should accomplish the same thing, in much less lines of code. The name mangling is achieved via a regex (same way as before), but this time is generalized for:

  • all Python "hard" keywords (see the docs for details)
  • all sympy top-level identifiers, except those that conflict with certain NMODL/HOC builtin functions (such as sin, cos, etc.)

Also sprinkled in some type hints.
Also added test_ode.py to coverage collection.

This allows us to build NEURON with NRN_ENABLE_NMODL=ON.

TODO

  • add tests for integrate2c (was not using [de]mangle_identifiers before)

@azure-pipelines

Copy link
Copy Markdown

✔️ 225cc45 -> Azure artifacts URL

@codecov

codecov Bot commented Jul 24, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.32%. Comparing base (f2d4ed3) to head (a8d33fc).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
share/lib/python/neuron/nmodl/ode.py 91.30% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3543   +/-   ##
=======================================
  Coverage   68.31%   68.32%           
=======================================
  Files         689      689           
  Lines      111038   111033    -5     
=======================================
- Hits        75861    75860    -1     
+ Misses      35177    35173    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

✔️ 225cc45 -> artifacts URL

@JCGoran
JCGoran marked this pull request as ready for review July 24, 2025 22:03
@github-actions

Copy link
Copy Markdown
Contributor

✔️ d9e3d17 -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ d9e3d17 -> Azure artifacts URL

@JCGoran
JCGoran requested a review from nrnhines August 4, 2025 10:27
@azure-pipelines

Copy link
Copy Markdown

✔️ 6059fe5 -> Azure artifacts URL

@github-actions

github-actions Bot commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

✔️ 6059fe5 -> artifacts URL

@github-actions

Copy link
Copy Markdown
Contributor

✔️ 8940991 -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ 8940991 -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ 2b2f315 -> Azure artifacts URL

@github-actions

Copy link
Copy Markdown
Contributor

✔️ 2b2f315 -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ b54d8d5 -> Azure artifacts URL

@github-actions

Copy link
Copy Markdown
Contributor

✔️ b54d8d5 -> artifacts URL

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

✔️ b611c56 -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ b611c56 -> Azure artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ 048fbbb -> Azure artifacts URL

@github-actions

Copy link
Copy Markdown
Contributor

✔️ 048fbbb -> artifacts URL

* Improve name-mangling functions (for sympy) and add unit test.

* More clean up for NMODL name-mangling and unit tests

* Make variables private (in share/lib/python/neuron/nmodl/ode.py)
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

✔️ a8d33fc -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ a8d33fc -> Azure artifacts URL

@nrnhines
nrnhines merged commit 5c586fb into master Jan 23, 2026
46 checks passed
@nrnhines
nrnhines deleted the jelic/fix_nmodl_name_mangling branch January 23, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider refactoring SymPy solver code in NMODL

3 participants