Optimize code generated for diam and area.

[1uc]

Optimizes nocmodl generated code to use the mechanism cache when accessing diam and area as described in #2913.

For performance testing I use:

NEURON {
  SUFFIX two_radii
  USEION ca READ cai
  NONSPECIFIC_CURRENT il
  RANGE y, inv
}

ASSIGNED {
  v
  y
  il
  inv
  diam
  area
  cai
}

INITIAL {
  y = 1.0*diam + area + cai
  inv = 1.0 / square_diam()
}

BREAKPOINT {
  il = square_diam() / inv * 0.001 * (v - 20.0)
}

FUNCTION square_diam() {
  square_diam = diam*diam + area
}

The name two_radii is to be able to grep for diam, we added the ion ca to compare with how it's solved for ions. The logic is to use inv to create a very expensive multiplication with 1.0.

The python file to measure the performance is:

import time

import numpy as np
from neuron import gui
from neuron import h

import matplotlib.pyplot as plt


nsec = 100
nseg = 1000

sections = [h.Section() for _ in range(nsec)]

for s in sections:
    s.nseg = nseg
    s.insert("two_radii")


t_hoc = h.Vector().record(h._ref_t)
v_hoc = h.Vector().record(sections[0](0.5)._ref_v)

h.stdinit()

t0 = time.time();
h.continuerun(50.0)
elapsed = time.time() - t0

print(f"{elapsed=}")

t = np.array(t_hoc.as_numpy())
v = np.array(v_hoc.as_numpy())

# plt.plot(t, v)
# plt.show()

Edit: I realized I was measuring with Debug. After switching to Release I needed to refine the measurement slightly (it now scales linearly with tstop). The time after fix is 5.6s and before 25s.

[azure-pipelines]

✔️ 2d75d7e -> Azure artifacts URL

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.22%. Comparing base (8cb9ec8) to head (a350cee).
Report is 4 commits behind head on master.

❗ Current head a350cee differs from pull request most recent head 0b45c4d

Please upload reports for the commit 0b45c4d to get more accurate results.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2914   +/-   ##
=======================================
  Coverage   67.22%   67.22%           
=======================================
  Files         569      569           
  Lines      104691   104702   +11     
=======================================
+ Hits        70378    70388   +10     
- Misses      34313    34314    +1     

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

[1uc]

@nrnhines @pramodk could you please check if it at least partly solves the performance issue?

[pramodk]

Here are the numbers:

I am using https://github.com/nrnhines/266806/ and with single thread . I am using Morphology_1/mod_files/cdp5StCmod.mod without nrnhines/266806@b3815a0 i.e. use diam directly :

## Master Branch
NEURON RUN took  30.94808864593506

   Path                   Min time/rank Max time/rank Avg time/rank Time %
      state-cdp5StCmod     18.409768     18.409768     18.409768   59.448159

## This PR

NEURON RUN took  21.428975582122803

   Path                   Min time/rank Max time/rank Avg time/rank Time %
      state-cdp5StCmod      9.139341      9.139341      9.139341   42.610451

And this time is similar to the one if we cache diam as a RANGE variable (as done in nrnhines/266806@b3815a0).

[pramodk]

LGTM from the performance comparison aspects.

[1uc]

While implementing the same solution in NMODL, I noticed, that calling function from Python leads to a SEGFAULT. I'm debugging, but more changes are needed for this PR.

This has now been fixed, essentially the m_offset for MechanismInstance was incorrect for dptr_field.

[azure-pipelines]

✔️ 7cb0aff -> Azure artifacts URL

[1uc]

@pramodk It now contains a fix for the SEGFAULT, I think it's principled, but it could use re-review. Sorry, CI seems to have gotten stuck, I'll rerun.

[azure-pipelines]

✔️ a350cee -> Azure artifacts URL

[pramodk]

@nrnhines : I have tested this and confirmed diam related perf regression is addressed. Do you to take a quick look at the change?

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[azure-pipelines]

✔️ 0b45c4d -> Azure artifacts URL

[nrnhines]

Great job. I've verified that this clearly fixes the performance issue with diam.
I tested with the sodium longitudinal diffusion example. Results

Init&Run time     version
1.9                      8.2.4
5.2.                     master
2.9                      thisPR
2.9                      thisPR with diam replaced by diamr range variable in KINETIC

The model files come from nrn/share/examples/nrniv/nmodl and the relevant files are

nacur.mod
ionleak.mod
nadifl.mod
nadifl2.hoc
nadifl2.ses

launch with

nrnivmodl
nrngui nadifl2.hoc

press the Init&Run button and note the RealTime at the bottom of the RunControl

[bbpbuildbot]

Logfiles from GitLab pipeline #218223 (:no_entry:) have been uploaded here!

Status and direct links:

• ✅ mac_m1_cmake_build: [cmake, ON, OFF, OFF, address]
• ✅ spack_setup
• ✅ build:nmodl
• ✅ build:neuron:nmodl:intel:legacy
• ✅ build:neuron:nmodl:intel:shared
• ✅ build:neuron:nmodl:nvhpc:acc:legacy
• ✅ build:neuron:nmodl:nvhpc:acc:shared
• ✅ build:neuron:nmodl:nvhpc:omp:legacy
• ✅ build:neuron:nmodl:nvhpc:omp
• ✅ test:neuron:nmodl:intel:legacy
• ✅ test:neuron:nmodl:intel:shared
• ✅ test:neuron:nmodl:nvhpc:acc:legacy
• ✅ test:neuron:nmodl:nvhpc:acc:shared
• ✅ test:neuron:nmodl:nvhpc:omp:legacy
• ✅ test:neuron:nmodl:nvhpc:omp

[pramodk]

I have neurodamus people about Gitlab failures. As this CI was green and failures are completely unrelated, I think we should skip/ignore gitlab CI here.

[1uc]

See #2787.