Allow segment.mechanism.func(args) by nrnhines · Pull Request #2475 · neuronsimulator/nrn

nrnhines · 2023-08-25T21:33:33Z

Closes #686
Includes #2504
Includes #2460

Allow mechanism function calls using the syntax section(x).mechanism.func(args) without requiring a setup call to h.setdata_mechanism(section(x)) for the case where func uses range variables.
E.g. the old syntax required:

h.setdata_hh(soma(.5))
h.rates_hh(-65)

and the new recommended syntax is

soma(.5).hh.rates(-65)

Need user documentation.
Need careful review and testing of try/catch for NPyMechFunc_call
Are data handles used when possible (instead of pointers)?
Is nrn.MechFunc sufficiently complete? Generically its methods are name() and mech(). A nrn.Mechanism presently has no provision for listing the FUNCTION and PROCEDURE available from the mod file, e.g. via dir(...).
An interesting feature of this implementation is that the call makes minimal use of HOC. (Just args on the hoc stack).

Timeit results from

$ cat temp2.py
import timeit

setup= '''
from neuron import h
s = h.Section()
s.insert("hh")
z = s(.5).hh.rates
zz = s(.5).hh.vtrap
'''

def tim(s):
    print(s, min(timeit.repeat(s, setup, number=10000)))

tim("s(.5).hh.rates(-30)")
tim("z(-30)")
tim("h.rates_hh(-30)")
tim("h.setdata_hh(s(.5));h.rates_hh(-30)")

tim("zz(0, 1)")

are

$ python3 temp2.py
s(.5).hh.rates(-30) 0.20718456991016865
z(-30) 0.0590008000144735
h.rates_hh(-30) 0.10944443196058273
h.setdata_hh(s(.5));h.rates_hh(-30) 0.3778606569394469
zz(0, 1) 0.05770030908752233

azure-pipelines · 2023-08-25T22:03:33Z

✔️ 077c083 -> Azure artifacts URL

codecov · 2023-08-25T22:17:01Z

Codecov Report

Merging #2475 (4574f28) into master (6004512) will decrease coverage by 0.14%.
The diff coverage is 96.69%.

@@            Coverage Diff             @@
##           master    #2475      +/-   ##
==========================================
- Coverage   65.98%   65.85%   -0.14%     
==========================================
  Files         560      559       -1     
  Lines      108876   108787      -89     
==========================================
- Hits        71844    71641     -203     
- Misses      37032    37146     +114

Files	Coverage Δ
src/nmodl/nocpout.cpp	`95.31% <100.00%> (+0.23%)`	⬆️
src/nmodl/parsact.cpp	`88.05% <100.00%> (+0.36%)`	⬆️
src/nmodl/parse1.ypp	`82.59% <100.00%> (+0.27%)`	⬆️
src/nrnoc/init.cpp	`89.94% <100.00%> (+0.08%)`	⬆️
src/nrnoc/membfunc.cpp	`66.66% <100.00%> (+1.28%)`	⬆️
src/nrnoc/membfunc.h	`100.00% <ø> (ø)`
src/nrnpython/nrnpy_hoc.cpp	`70.94% <100.00%> (ø)`
src/nrnpython/nrnpy_utils.h	`86.73% <ø> (ø)`
src/nrnpython/nrnpy_nrn.cpp	`74.60% <93.04%> (+1.60%)`	⬆️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…ier_without_container Allows checking of _extcall_prop validity by shadowing with an id.

in same fashion as vectorized mod files.

azure-pipelines · 2023-09-16T13:02:27Z

✔️ 6bb061e -> Azure artifacts URL

Factorization (incomplete) of new NPyMechObj with new_pymechobj. Unsolved tests failures: 21 - pytest_coreneuron::basic_tests_py3.10 (Failed) 102 - coreneuron_modtests::datareturn_py_cpu (Failed)

azure-pipelines · 2023-10-13T19:38:12Z

✔️ 0ab60ff -> Azure artifacts URL

pramodk · 2023-10-13T23:44:34Z

apple m1 failure seems unrelated. Will take a look tomorrow.

pramodk

LGTM

Gitlab CI launched internally for Apple M1 has passed: https://bbpgitlab.epfl.ch/hpc/cellular/nrn/-/jobs/965444

sonarqubecloud · 2023-10-16T18:15:51Z

SonarCloud Quality Gate failed.

24 Bugs
0 Vulnerabilities
0 Security Hotspots
61 Code Smells

No Coverage information
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

azure-pipelines · 2023-10-16T18:52:51Z

✔️ 4574f28 -> Azure artifacts URL

ramcdougal · 2023-10-16T18:57:45Z

Is nrn.MechFunc sufficiently complete? Generically its methods are name() and mech(). A nrn.Mechanism presently has no provision for listing the FUNCTION and PROCEDURE available from the mod file, e.g. via dir(...).

With respect to our conversation with @Helveg earlier today, nrn.hh ought to provide a way of getting the generic density mechanism h.hh (and similarly for all density mechanisms)

bbpbuildbot · 2023-10-16T19:30:34Z

Helveg · 2023-10-17T10:27:36Z

@ramcdougal @nrnhines I suppose that Robert's latest comment hasn't been addressed in this PR? Equality checks also fail between nrn.Mechanisms

nrnhines · 2023-10-17T10:39:24Z

@ramcdougal @Helveg

equality, access to generic type.

That is an oversight on my part. I'll start another PR. Does anyone offhand know if there is an issue to be associated with it?
Is it worthwhile to extend equality to mechanism functions?

Helveg · 2023-10-17T10:52:52Z

We didn't open an issue yet, just the Slack thread and comments here.

I use equality checks to avoid circular logic by keeping a bag of already visited NEURON objects in a differ that I'm writing for NEURON that reports differences between 2 objects recursively visiting everything attached/related to the input objects. Generally speaking, anything that leads to a more complete Python interface would allow Python tooling to write cleaner and simpler code.

Right now I'm monkey patching the __hash__ and __eq__ methods, but I'm sure I'll run into a readonly class object soon enough ;)

ramcdougal · 2023-10-17T13:09:30Z

I was going to object to your use of "more complete" because you can get all the information you seek through either sec.psection() directly or MechanismStandard or MechanismType... But yeah, that's fair, if equality doesn't work that is incomplete.

nrnhines added 5 commits August 14, 2023 07:01

Avoid segfault if user fails to call setdata before a mech FUNCTION.

fc67786

Edit in response to comments. Rudimentary test.

e00bad0

Merge branch 'master' into hines/setdata

510e072

Allow segment.mechanism.func(args).

262b299

Merge branch 'master' into hines/mechfunc

077c083

This comment has been minimized.

Sign in to view

nrnhines added 16 commits August 26, 2023 08:35

Merge branch 'master' into hines/setdata

6174b37

Merge branch 'master' into hines/setdata

a75659b

more generic test framework for sdata

3adc3d4

Merge branch 'master' into hines/setdata

600a4ab

_nrn_get_prop_id(Prop*) returns neuron::container::non_owning_identif…

d107912

…ier_without_container Allows checking of _extcall_prop validity by shadowing with an id.

Don't use neuron or container name tokens after user names are defined.

cd8d40e

Vectorized mod file functions that do not use RANGE always work.

ac958f7

Merge branch 'master' into hines/setdata

35cd480

Fixed mnod file array variable assignment (seg.mech.array[i] = x)

9fc95a9

Non vectorized mod file functions with and without RANGE work

dbfd765

in same fashion as vectorized mod files.

Merge branch 'master' into hines/setdata

0d751fe

Properly handle "SUFFIX nothing" for func_needs_setdata

91b9ee2

If no RANGE then no need for prior setdata call.

0c4adaf

Merge branch 'master' into hines/mechfunc

d56c512

Merge branch 'hines/fix-modarray-assign' into hines/mechfunc

25c8943

Merge branch 'hines/setdata' into hines/mechfunc

6bb061e

This comment has been minimized.

Sign in to view

nrnhines mentioned this pull request Sep 16, 2023

Avoid segfault if user fails to call setdata before a mech FUNCTION. #2460

Closed

nrnhines added 3 commits September 17, 2023 13:04

seg.mech.fun(args) working when Prop* is valid.

5309078

Err instead of segfault if Point process or mech instance not valid.

b010b17

Merge branch 'master' into hines/mechfunc

e453696

nrnhines marked this pull request as draft October 3, 2023 09:51

This comment has been minimized.

Sign in to view

segment.mech.__dict__ also contains FUNCTION,PROCEDURE names

c27aeb3

This comment has been minimized.

Sign in to view

Printing the name of an object should never be an error.

72210ca

Factorization (incomplete) of new NPyMechObj with new_pymechobj. Unsolved tests failures: 21 - pytest_coreneuron::basic_tests_py3.10 (Failed) 102 - coreneuron_modtests::datareturn_py_cpu (Failed)

This comment has been minimized.

Sign in to view

nrnhines added 3 commits October 5, 2023 09:02

mech_of_seg_next first call returns first item

d76c575

Merge branch 'hines/setdata' into hines/mechfunc

313197f

Merge branch 'master' into hines/mechfunc

008b00c

This comment has been minimized.

Sign in to view

Merge branch 'master' into hines/mechfunc

0ab60ff

This comment has been minimized.

Sign in to view

nrnhines marked this pull request as ready for review October 13, 2023 21:46

pramodk approved these changes Oct 16, 2023

View reviewed changes

Comment thread src/nmodl/nocpout.cpp

Comment thread src/nrnpython/nrnpy_nrn.h Outdated

Comment thread test/pytest_coreneuron/test_basic.py Outdated

Resolve conversations. Test FUNCTION_TABLE

4574f28

nrnhines merged commit 6cb0bac into master Oct 16, 2023

nrnhines deleted the hines/mechfunc branch October 16, 2023 23:31

This was referenced Jan 11, 2024

Add semantic analysis check for infinite loops with function calls BlueBrain/nmodl#1138

Closed

Setting use_range_ptr_var NmodlType in SymTab BlueBrain/nmodl#1139

Merged

nrnhines mentioned this pull request May 29, 2024

Investigate segfault when calling h.<function>_<suffix> BlueBrain/nmodl#1283

Open

Uh oh!

Conversation

nrnhines commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Aug 25, 2023

Uh oh!

codecov Bot commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

azure-pipelines Bot commented Sep 16, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

azure-pipelines Bot commented Oct 13, 2023

Uh oh!

This comment has been minimized.

pramodk commented Oct 13, 2023

Uh oh!

pramodk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Oct 16, 2023

Uh oh!

azure-pipelines Bot commented Oct 16, 2023

Uh oh!

ramcdougal commented Oct 16, 2023

Uh oh!

bbpbuildbot commented Oct 16, 2023

Uh oh!

Helveg commented Oct 17, 2023

Uh oh!

nrnhines commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Helveg commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ramcdougal commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nrnhines commented Aug 25, 2023 •

edited

Loading

codecov Bot commented Aug 25, 2023 •

edited

Loading

nrnhines commented Oct 17, 2023 •

edited

Loading

Helveg commented Oct 17, 2023 •

edited

Loading

ramcdougal commented Oct 17, 2023 •

edited

Loading