Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate make_mechanism and make_pointprocess from NEURON #2691

Closed
nrnhines opened this issue Jan 28, 2024 · 6 comments
Closed

Eliminate make_mechanism and make_pointprocess from NEURON #2691

nrnhines opened this issue Jan 28, 2024 · 6 comments
Assignees
Milestone

Comments

@nrnhines
Copy link
Member

I propose elimination of make_mechanism and make_pointprocess. See documentation at https://nrn.readthedocs.io/en/latest/python/modelspec/programmatic/hocmech.html

This feature does not work with NEURON version 9.

This feature is used in modeldb only by https://modeldb.science/9853 implemented by me. hhint and hhq are trivially implementable with nmodl.

I believe it is not worth the effort to fit this feature into the SoA style of NEURON version 9. And elimination will remove a good deal of special case and confusing code.

@nrnhines nrnhines self-assigned this Jan 28, 2024
@nrnhines nrnhines added this to the Release v9.0 milestone Jan 28, 2024
@nrnhines
Copy link
Member Author

I was mistaken about https://modeldb.science/9853 hhint and hhq mechanisms being trivially implementable. I still think they are implementable but, as they interact with other inserted mechanisms, require a good deal of extra implementation effort, such as USEION (not so bad) and POINTER variables (extra effort on the hoc side to set them up). I am also possibly mistaken about the effort needed to bring make_mechanism into compatibility with NEURON version 9 (just a few lines of registration code). Unfortunately, the 9853 model figures run about 20 times slower in version 9 than in 8.2.3 (about 10 seconds as opposed to 0.5 seconds). (That model has 100 segments and with variable step method needs about 2000 time steps) Figure 4 runs 60 times slower. I don't know the reason for the performance issue. But I'd like to diagnose and fix that, keep these functions, and withdraw this proposal.

@nrnhines
Copy link
Member Author

It is interesting that the latest #2675 with the default compile of -DCMAKE_BUILD_TYPE=RelWithDebInfo runs the slowest (Fig 4) of https://modeldb.science/9853 in 0.9s as opposed to 30s with Debug. However the 8.2.3 version (RelWithDebInfo) is still a quite a bit faster at 0.5s . So there is still some curiosity as to the reason for the performance difference. This machine is
Intel® Xeon(R) CPU E5606 @ 2.13GHz × 8 processor.

@1uc
Copy link
Collaborator

1uc commented Feb 1, 2024

In scientific codes that use a lot of templates and rely heavily on inlining, -O0 can be prohibitively slow. We have a different build type which should maintain a balance between speed:

# FastDebug: Similar to Debug with a bit higher level optimisations (-O1) and other compiler
# flags so that it's faster than -O0 but still produces consistent results for
# testing and debugging purposes.

Is the difference similarly big when using -DCMAKE_BUILD_TYPE=FastDebug?

@nrnhines
Copy link
Member Author

FastDebug is not allowed if InterViews is part of the build:

CMake Error at external/iv/CMakeLists.txt:46 (message):
  Invalid build type: FastDebug : Must be one of
  Custom;Debug;Release;RelWithDebInfo

Actually, I'm more concerned about why 8.2.3 is twice as fast (RelWithDebInfo)

@1uc
Copy link
Collaborator

1uc commented Feb 13, 2024

We should have a look with a profiler. If we're lucky it's something trivial, if not it's still good to know why and check it doesn't affect larger simulations.

I've modified the CMake code for IV to allow it to build with FastDebug. We'll need a PR here, updating the SHA and review why CI doesn't catch this build failure.

@nrnhines
Copy link
Member Author

With -DCMAKE_BUILD_TYPE=FastDebug now working for the InterViews portion of the build, neuronsimulator/iv#50 ,
the performance is similar to RelWithDebInfo.
Given the name of this Issue, I'm closing it. The half speed relative to 8.2.3 can be dealt with separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants