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

Merge NMODL CMakeLists.txt #3272

Draft
wants to merge 909 commits into
base: master
Choose a base branch
from
Draft

Merge NMODL CMakeLists.txt #3272

wants to merge 909 commits into from

Conversation

1uc
Copy link
Collaborator

@1uc 1uc commented Dec 5, 2024

No description provided.

JCGoran and others added 30 commits June 3, 2024 10:32
* Remove INCLUDE mod file generation in tests

- include mod files generation cause race condition with parallel tests, see BlueBrain/nmodl#1290
- the included files are trivial examples for testing and they don't need to be
  generated from the code
  - could be copied via configure_file
  - cmake could generate those at a configure time

Considering the use case and it's triviality, I thought 2nd option is better. That
also allows to remove some extra code/logic.

NMODL Repo SHA: BlueBrain/nmodl@844e6bd
---------

Co-authored-by: Nicolas Cornu <[email protected]>

NMODL Repo SHA: BlueBrain/nmodl@751377d
The new structure is to have one directory per keyword. Tests can be organized in separate test files, named test_*.py (or simulate.py if that's a better name).

Numerous tests have been polished to make them easier to read, using better names and functions to extract common functionality.

NMODL Repo SHA: BlueBrain/nmodl@cc44ecb
The executable from the Python library.

NMODL Repo SHA: BlueBrain/nmodl@c5ecfea
The changes are:

* Don't forcefully set the PYTHONPATH.
* Replace false object oriented code with a function (each).
* Replace a global variable, with a function to initialize.
* Use a reference for a pointer that's should never be a nullptr.



NMODL Repo SHA: BlueBrain/nmodl@2636b4e
Frequently `_lmr` is passed by pointer, which requires printing:
```
  foo(&_lmr, ...);
```
the `&` is annoying, because it prevents us from using `get_arg_str`. It
also sometimes leads us to do:
```
  auto * _ml = &_lmr;
```
which is bad, because it's not a `Memb_list`.

* Rename '_lmr' to '_lmc'.

NMODL Repo SHA: BlueBrain/nmodl@13550d7
There must not be a clash of FUNCTION and RANGE names.

NMODL Repo SHA: BlueBrain/nmodl@d226bef
These are global variables, and when THREAD_SAFE act like thread
variables, without being exposed to HOC/Python.

NMODL Repo SHA: BlueBrain/nmodl@e9e127a
Similar to BlueBrain/nmodl#1305, include the solver headers in variables in a C++
header, and include them in the code generated for the MOD files. This
should allow to substitute different NMODL binaries in NEURON builds
without confusion as to which solver headers are going to be included in
the MOD file compilation.

NMODL Repo SHA: BlueBrain/nmodl@7910146
Copy link

✔️ e05104d -> Azure artifacts URL

@pramodk
Copy link
Member

pramodk commented Jan 22, 2025

@JCGoran : If we want to finalize this PR (and not #3265), could we add task list here listing all pending todos?

(Finally I have development machine setup and I could help to finalize this PR)

@JCGoran JCGoran force-pushed the 1uc/nmodl-merge-cmake branch from 630f11c to 90e02ff Compare January 22, 2025 17:23
@JCGoran
Copy link
Collaborator

JCGoran commented Jan 23, 2025

Hey @pramodk, glad to have you back! I think I got most of the way there as far as the CMake integration is concerned. I suspect the 3 failing tests are related to this snippet:

if(NOT LINK_AGAINST_PYTHON)
target_link_libraries(${test_name} PRIVATE pywrapper ${PYTHONLIB})
endif()

I.e. instead of PYTHONLIB (found via find_libpython) it should use one of the CMake variables that NEURON uses for the default Python version.

I will be away for the next couple of weeks so it'd be great if you could work on this.

Copy link

✔️ 9f8624f -> Azure artifacts URL

Copy link

✔️ 3ceeec0 -> Azure artifacts URL

Copy link

✔️ dce10e9 -> Azure artifacts URL

Copy link

✔️ 058ab43 -> Azure artifacts URL

Copy link

✔️ a2023c9 -> Azure artifacts URL

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

Successfully merging this pull request may close these issues.

NMODL integration
6 participants